diff --git a/.claude/references/compact-style.md b/.claude/references/compact-style.md new file mode 100644 index 0000000000..902cb70855 --- /dev/null +++ b/.claude/references/compact-style.md @@ -0,0 +1,104 @@ +# Compact style for issues and PR bodies + +Shared reference. Pointed at by `.claude/skills/jira-issue/SKILL.md` and +`.claude/skills/pr-pitch/SKILL.md`. Adapted from the MIT-licensed +`i-have-adhd` skill (https://github.com/ayghri/i-have-adhd), which shapes chat +turns; these rules shape written artifacts instead. + +## What changes when the reader is not in the conversation + +A chat reader has just asked you something. A Jira reader is scanning a queue +of forty tickets, and a PR reader has eleven other reviews open. Both arrive +cold, months after the work, with none of your context and no way to ask a +follow-up. Four consequences: + +1. The first line is the only line guaranteed to be read. It carries the + decision, not the setup. +2. Nothing can be held in the reader's head from a previous paragraph. +3. There is no "let me know if you need anything else". Whatever is unanswered + stays unanswered. +4. Length is not a cost the reader chooses to pay. Jira Data Center has no + collapsible sections, so every word in a description is on the screen. + +## The rules + +### 1. Lead with what the reader must decide + +Not context, not history, not the investigation. For a triager: what breaks, +who it hits, how bad. For a reviewer: what the change does and what it risks. + +Bad: `h3. The underlying problem` / "While investigating LT-22710 we noticed" +Good: "Deleting a feature-based natural class empties every rule that used it." + +### 2. Number every multi-step sequence + +Repro steps, verification steps, migration steps. One bounded action per step. +**No step contains "and then" twice.** If a step needs a sub-list, it is two +steps. + +### 3. End with one named next action + +Every description ends with a single `*Next:*` line naming who does what. +Tickets and PRs that end without one stall, because nobody is named. + +Good: `*Next:* reporter to confirm the FLEx version (see comment).` +Good: `Next: approve, or tell me to split the installer change out.` + +### 4. One artifact, one problem + +A ticket describing four problems is four tickets, linked. A PR doing three +unrelated things is three PRs, stacked. Splitting is cheap at filing time and +expensive at review time. + +### 5. State cause, not concern + +No "seems to", no "there may be an issue with", no apology. Name the symptom +and, if known, the mechanism. If the mechanism is inferred rather than +observed, say "inferred" -- that is information, whereas hedging is noise. + +### 6. Cost shape, not hours + +Hours on a ticket read as a commitment nobody made. Size the work by what it +touches. + +Bad: "This will take some work." / "About two days." +Good: "One branch in one file." / "Needs a liblcm release and a package bump." + +### 7. Cap any list at five + +Past five, split into "must" and "nice to have", or accept that the artifact +is really an epic. Five ranked beats ten unranked. + +### 8. No preamble, no recap, no closer + +Banned openers: "This issue describes", "This PR refactors", "While +investigating", "As a note", "I have been looking into". Banned closers: +"Please let me know", "Hope this helps", "Happy to provide more detail". + +Start with the answer. Stop when the answer is done. + +### 9. Say what is not known + +An explicit `*Not known:*` line is worth more than a confident guess. It tells +the next reader where to dig and stops a fabrication becoming folklore. + +### 10. Never assert what was not verified + +Everything above the fold is either something a reporter said or something we +observed. Analysis, inference and reconstruction go in a comment or an +accordion, labelled as such. A screenshot from a headless test is not a +screenshot of the product; say which it is. + +## Pre-send check + +Delete before publishing: + +1. The first sentence, if it announces what the artifact is about to do. +2. The last sentence, if it recaps or asks for further questions. +3. Any "by the way" sidebar. It is a separate ticket. +4. Hedging adverbs carrying no information. Keep a hedge that carries real + uncertainty; deleting that one manufactures confidence. +5. Any idiom. Replace with the literal action. + +Then verify: **reading only the title and the last line, does the reader know +what is wrong and what happens next?** If yes, publish. diff --git a/.claude/references/evidence.md b/.claude/references/evidence.md new file mode 100644 index 0000000000..fa7888a724 --- /dev/null +++ b/.claude/references/evidence.md @@ -0,0 +1,133 @@ +# Screenshots and visual evidence + +Shared reference. Pointed at by `.claude/skills/pr-pitch/SKILL.md`, +`.claude/skills/jira-issue/SKILL.md` and +`.claude/skills/fieldworks-avalonia-ui/SKILL.md`. + +Three stages, and skipping the middle one is the usual failure: +**capture, curate, publish.** + +## The rule that outranks the rest + +**The test is the evidence. The screenshot is the courtesy.** + +A headless capture proves nothing a reviewer can re-run. The assertion does. +So a PR carries the test name *and* the picture, and the picture never carries +a claim the test does not. + +The corollary: **a control-level headless capture is not a screenshot of the +product.** If nothing writes the operation at runtime yet, the image shows a +renderer, not a feature. Say which it is, in the caption, every time. That +distinction evaporates the moment an image is pasted without one. + +## Capture + +| Surface | Who captures | How | +| --- | --- | --- | +| Avalonia | The agent, automated | Headless Skia. See `.claude/skills/fieldworks-avalonia-ui/references/visual-snapshot-testing.md` | +| WinForms, live project | The developer | Real scenarios need real data, and real data needs permission | +| WinForms, throwaway project | Either | `fieldworks-winapp/navigation/screenshot-evidence.md`, MCP-driven | + +Captures land in `Output/ManualEvidence//NN-name.png`. That directory +is gitignored, which is correct -- captures are working output, not artifacts. + +## Curate + +An uncaptured curation step is why evidence reads as decoration. Three things: + +**Trim to content.** Renderer captures are mostly background. A 520x180 capture +with content in the top quarter reads as an empty box at thumbnail size, which +is the size it is first seen at in both GitHub and Jira. Crop to the content +bounds plus about 8 pixels. + +**Caption every image.** What to look at, not what it is. "Before" is not a +caption; "Before -- the Lexeme field lists both seh and pt" is. + +**Label provenance in the caption.** One of: headless control-level capture, +live FLEx desktop, or mockup. Never leave it to be inferred. + +Name files `NN-state-subject.png` so they sort into reading order: +`01-before-writing-systems.png`, `02-after-writing-systems.png`. + +## Publish -- GitHub + +Try these in order and say which one was used. + +**1. `gh --attach`, once it ships.** Native upload on six commands (issue and +PR create, edit, comment), tracked by `github/roadmap#1324`. It uses the +ordinary `gh` token, so no cookie and no committed file. Constraints: write +access required, **Actions tokens excluded** so CI cannot use it, nine file +types, images under 10 MB. Detect it rather than assuming a version: + +```powershell +if ((gh pr comment --help 2>&1 | Out-String) -match '--attach') { "native upload available" } +``` + +**2. `gh image`** (`drogers0/gh-image`, MIT). Drives the web UI's own upload +flow and returns a real `user-attachments` URL. It needs a GitHub **session +cookie**, not the `gh` token: `--token`, `GH_SESSION_TOKEN`, or extraction +from a browser cookie store. Chrome 127 and later encrypt cookies in a way +that defeats extraction on Windows, so a Chrome-only machine will report +`session token is empty`. + +- Check availability with `gh image check-token`, which prints a username. +- **Never run `gh image extract-token` in an agent session.** It prints a + full-account credential to stdout, and stdout becomes conversation context. +- A `user_session` cookie grants complete account access and bypasses 2FA. If + a developer chooses this route, they set `GH_SESSION_TOKEN` in their own + shell before starting the session -- never pasted into a prompt. + +**3. Ask the author to drag it in.** When neither route above is available -- +no `--attach` yet, no session cookie, or a CI run, where the official flag +excludes Actions tokens anyway -- say so and hand the file over. Name the exact +path to drop into the comment box, then splice the returned URL into the body. + +That is a real answer, not a failure. An agent that cannot upload should say +which route it tried and stop, rather than inventing somewhere to put the file. + +**Do not commit images to the repository** to work around this, and do not +create a side branch to host them. Both put binaries in history permanently to +solve a problem that lasts one review. + +## Publish -- Jira + +Jira takes native attachments, which is better than a URL there because they +outlive any branch: + +```powershell +python -c @' +import sys; sys.path.insert(0, ".claude/skills/atlassian-skills/scripts") +from jira_attachments import jira_add_attachment +print(jira_add_attachment("LT-22715", ["01-before.png", "02-after.png"])) +'@ +``` + +Then reference them from the description or comment with `!01-before.png!`, +or `!01-before.png|thumbnail!` to keep a long description scannable. Images +belong in the analysis comment unless the picture *is* the bug report. + +## Permission + +**Hard stop, every time, before anything leaves the machine:** + +> Do you have permission to post this? + +A screenshot of a live project is a data disclosure exactly as a sample +project is: vernacular text, speaker names, unpublished lexical data, +community-owned material. Jira attachments are visible to everyone with +project access, and a GitHub attachment on a public repo is public. + +- Never publish a capture the agent found on disk without being told to. +- Agent-captured WinForms evidence comes from a throwaway test project only. +- If permission is unclear, describe the image instead and say in the ticket + that a capture exists but was not attached, so nobody re-asks. + +## Checklist + +- [ ] The claim the image supports is also pinned by a test, or the image is + labelled as the only evidence. +- [ ] Trimmed to content. +- [ ] Captioned with what to look at. +- [ ] Provenance named: headless, live, or mockup. +- [ ] Permission asked and answered before upload. +- [ ] The publish route used is stated, including when it was you. diff --git a/.claude/skills/atlassian-skills/SKILL.md b/.claude/skills/atlassian-skills/SKILL.md index ed05cb53b7..7f44b60592 100644 --- a/.claude/skills/atlassian-skills/SKILL.md +++ b/.claude/skills/atlassian-skills/SKILL.md @@ -281,6 +281,15 @@ from scripts.jira_issues import ( jira_add_comment # Add comment to issue ) +# Attachments live in their own module because they need a multipart POST +from scripts.jira_attachments import ( + jira_add_attachment # Upload one or more files to an issue +) + +# Attachments are visible to everyone who can see the issue. Confirm +# permission before uploading user data -- see .claude/references/evidence.md +jira_add_attachment("LT-22715", ["01-before.png", "02-after.png"]) + # Create issue with full options jira_create_issue( project_key="PROJ", diff --git a/.claude/skills/atlassian-skills/scripts/jira_attachments.py b/.claude/skills/atlassian-skills/scripts/jira_attachments.py new file mode 100644 index 0000000000..e51e8921dd --- /dev/null +++ b/.claude/skills/atlassian-skills/scripts/jira_attachments.py @@ -0,0 +1,139 @@ +"""Jira attachment tools. + +Tools: + - jira_add_attachment: Upload one or more files to an issue + +Attachments need a multipart POST, which AtlassianClient.post cannot do -- +it only sends JSON. This module therefore drives client.session directly, +reusing the client's base URL, auth, SSL setting and error handling. +""" + +import mimetypes +import os +import sys +from pathlib import Path +sys.path.insert(0, str(Path(__file__).parent)) + +from typing import Any, Dict, List, Optional, Union + +from _common import ( + AtlassianCredentials, + get_jira_client, + format_json_response, + format_error_response, + ConfigurationError, + AuthenticationError, + ValidationError, + NotFoundError, + APIError, + NetworkError, +) + +# Jira Data Center's default ceiling. A larger file fails server-side with a +# message that does not name the limit, so check it here where we can say so. +DEFAULT_MAX_BYTES = 10 * 1024 * 1024 + + +def jira_add_attachment( + issue_key: str, + file_paths: Union[str, List[str]], + credentials: Optional[AtlassianCredentials] = None, + max_bytes: int = DEFAULT_MAX_BYTES +) -> str: + """Attach one or more files to a Jira issue. + + Args: + issue_key: Issue key (e.g., 'LT-22715') + file_paths: A path, or a list of paths, to upload + credentials: Optional AtlassianCredentials for Agent environments. + If not provided, uses environment variables. + max_bytes: Reject any file larger than this before uploading + + Returns: + JSON string with one entry per attachment, each carrying id, + filename, size and the content URL, or error information + + Note: + Attachments are visible to everyone who can see the issue. Confirm + permission to publish before calling this with user data -- + FieldWorks projects, screenshots of live data, and logs frequently + contain unpublished language material. + """ + handles = [] + try: + client = get_jira_client(credentials) + + if not issue_key: + raise ValidationError('issue_key is required') + if not file_paths: + raise ValidationError('at least one file path is required') + + if isinstance(file_paths, str): + file_paths = [file_paths] + + for path in file_paths: + if not os.path.isfile(path): + raise ValidationError(f'file not found: {path}') + size = os.path.getsize(path) + if size == 0: + raise ValidationError(f'file is empty: {path}') + if size > max_bytes: + raise ValidationError( + f'file is {size} bytes, over the {max_bytes} byte limit: {path}' + ) + + files = [] + for path in file_paths: + name = os.path.basename(path) + mime = mimetypes.guess_type(name)[0] or 'application/octet-stream' + handle = open(path, 'rb') + handles.append(handle) + files.append(('file', (name, handle, mime))) + + url = f"{client.config.url}{client.api_path(f'issue/{issue_key}/attachments')}" + + # X-Atlassian-Token defeats Jira's XSRF check, which otherwise rejects + # the upload. Content-Type must be cleared so requests can set the + # multipart boundary; the session sets application/json for every + # other call, and a None value here removes it for this one. + response = client.session.post( + url, + files=files, + headers={'X-Atlassian-Token': 'no-check', 'Content-Type': None}, + timeout=120, + verify=client.ssl_verify + ) + client._handle_error(response) + + uploaded: List[Dict[str, Any]] = [] + for item in (response.json() if response.content else []): + uploaded.append({ + 'id': item.get('id', ''), + 'filename': item.get('filename', ''), + 'size': item.get('size', 0), + 'mimeType': item.get('mimeType', ''), + 'content': item.get('content', ''), + 'thumbnail': item.get('thumbnail', '') + }) + + return format_json_response({ + 'issue_key': issue_key, + 'count': len(uploaded), + 'attachments': uploaded + }) + + except ConfigurationError as e: + return format_error_response('ConfigurationError', str(e)) + except AuthenticationError as e: + return format_error_response('AuthenticationError', str(e)) + except ValidationError as e: + return format_error_response('ValidationError', str(e)) + except NotFoundError as e: + return format_error_response('NotFoundError', str(e)) + except (APIError, NetworkError) as e: + return format_error_response(type(e).__name__, str(e)) + except Exception as e: + return format_error_response('UnexpectedError', f'Unexpected error: {str(e)}') + finally: + for handle in handles: + handle.close() diff --git a/.claude/skills/fieldworks-avalonia-ui/SKILL.md b/.claude/skills/fieldworks-avalonia-ui/SKILL.md index 09c5b64e6b..be0bbc9a3a 100644 --- a/.claude/skills/fieldworks-avalonia-ui/SKILL.md +++ b/.claude/skills/fieldworks-avalonia-ui/SKILL.md @@ -173,6 +173,29 @@ Rules specific to dialogs: `../fieldworks-winforms-to-avalonia-migration/references/parity-evidence.md` §"Evidence language"). +## Evidence for a PR or a ticket + +Avalonia is the surface where capture is automated, so a visible change ships +with a picture. Do not leave it to the reviewer to imagine the before and +after. + +1. Capture both states from a permanent headless test, not a throwaway + fixture, so the evidence regenerates. `references/visual-snapshot-testing.md` + has the harness. +2. Assert the behaviour deterministically in that same test. **The test is the + evidence; the screenshot is the courtesy.** Keep PNGs as subjective + evidence rather than pixel-golden tests. +3. Trim, caption and label before publishing, then upload by the routes in + `.claude/references/evidence.md`. + +Label every capture **control-level headless**, never "screenshot of FLEx", +unless the product actually drives the code path. When an operation exists but +nothing writes it at runtime yet, say so beside the image; a reader who +assumes otherwise believes a feature has shipped. + +Captures belong in `Output/ManualEvidence//`, which is gitignored. +Do not commit files from `Output`. + ## Handoff Report Avalonia docs consulted, tests run, remaining prototype gaps, diff --git a/.claude/skills/fieldworks-winapp/navigation/winforms-avalonia-parity.md b/.claude/skills/fieldworks-winapp/navigation/winforms-avalonia-parity.md index 4bcc24c8a4..90d696e715 100644 --- a/.claude/skills/fieldworks-winapp/navigation/winforms-avalonia-parity.md +++ b/.claude/skills/fieldworks-winapp/navigation/winforms-avalonia-parity.md @@ -17,7 +17,7 @@ a dialog, a browse table. The WinForms side needs the live app (this skill); the - A project with the relevant data (e.g. Words ▸ Analyses parity needs **parsed wordform analyses** — a bare project shows an empty interlinear). Restore per `project-loading.md`. - Evidence folders and parity-bundle naming: see `screenshot-evidence.md` (canonical parity - layout defined in the migration skill's `references/parity-evidence.md` §6). + layout defined in the migration skill's `.claude/skills/fieldworks-winforms-to-avalonia-migration/references/parity-evidence.md` §6). ## The two captures diff --git a/.claude/skills/fieldworks-winapp/references/research.md b/.claude/skills/fieldworks-winapp/references/research.md index 7d50737a67..03644c587e 100644 --- a/.claude/skills/fieldworks-winapp/references/research.md +++ b/.claude/skills/fieldworks-winapp/references/research.md @@ -51,7 +51,7 @@ sequence, cues, and safety notes to perform that route. - One reusable navigation destination or workflow per `navigation/*.md` file. - Shared safety and triggering rules stay in `SKILL.md`. -- Skill maintenance rules stay in `references/how-to-update.md`. +- Skill maintenance rules stay in `how-to-update.md`. - Research rationale stays in this file so future edits can revisit the design without bloating the active route instructions. diff --git a/.claude/skills/fieldworks-winforms-to-avalonia-migration/references/architecture-patterns.md b/.claude/skills/fieldworks-winforms-to-avalonia-migration/references/architecture-patterns.md index 29ba03d9d8..7d61375935 100644 --- a/.claude/skills/fieldworks-winforms-to-avalonia-migration/references/architecture-patterns.md +++ b/.claude/skills/fieldworks-winforms-to-avalonia-migration/references/architecture-patterns.md @@ -367,7 +367,7 @@ delta in the region manifest). > intentionally diverge. The visual-parity evidence type therefore checks > density/layout, not pixel-for-pixel appearance. The density tokens and > per-view border/font rules that this parity is measured against live in -> `fieldworks-avalonia-ui/references/style-system.md`, even where styling +> `.claude/skills/fieldworks-avalonia-ui/references/style-system.md`, even where styling > intentionally diverges. **Canonical code.** `Src/Common/FwAvalonia/FwAvaloniaDensity.cs`; diff --git a/.claude/skills/fieldworks-winforms-to-avalonia-migration/references/control-exemplar-map.md b/.claude/skills/fieldworks-winforms-to-avalonia-migration/references/control-exemplar-map.md index 1fa613fc38..80762f0351 100644 --- a/.claude/skills/fieldworks-winforms-to-avalonia-migration/references/control-exemplar-map.md +++ b/.claude/skills/fieldworks-winforms-to-avalonia-migration/references/control-exemplar-map.md @@ -80,7 +80,7 @@ headless tests), and then PROMOTES it: citations. 2. Add a numbered subsection to dialog-conversion.md (the §2c/§2d shape: what the legacy behavior was, the shared design, the test names). -3. Record any surprise in `references/lessons-learned.md` per its update +3. Record any surprise in `lessons-learned.md` per its update protocol. Until a gap's exemplar exists, conversions that need it stay on the Legacy diff --git a/.claude/skills/fieldworks-winforms-to-avalonia-migration/references/lessons-learned.md b/.claude/skills/fieldworks-winforms-to-avalonia-migration/references/lessons-learned.md index c7662b1bff..81b3f20fc1 100644 --- a/.claude/skills/fieldworks-winforms-to-avalonia-migration/references/lessons-learned.md +++ b/.claude/skills/fieldworks-winforms-to-avalonia-migration/references/lessons-learned.md @@ -248,8 +248,8 @@ requirements it fed are synced to `openspec/specs/lexical-edit-parity-automation `xWorksTests` (~1400 tests share the host); the restored test base holds the undoable task open (no nested `NonUndoableUnitOfWorkHelper`); `OnChangeFilter` takes an (added, removed) delta that `RecordList` composes into its `AndFilter`. -- Skill files changed: `references/architecture-patterns.md` (§13), - `references/parity-evidence.md` (§2a, §3), `references/migration-checklist.md` +- Skill files changed: `architecture-patterns.md` (§13), + `parity-evidence.md` (§2a, §3), `migration-checklist.md` (Phase 7), `SKILL.md` (quick map + workflow step 7), this ledger. ### 2026-06 — Lexical Edit (full entry view), phases 1–2 (seed entry) diff --git a/.claude/skills/fieldworks-winforms-to-avalonia-migration/references/migration-checklist.md b/.claude/skills/fieldworks-winforms-to-avalonia-migration/references/migration-checklist.md index e312758cfd..c5b5cdec1d 100644 --- a/.claude/skills/fieldworks-winforms-to-avalonia-migration/references/migration-checklist.md +++ b/.claude/skills/fieldworks-winforms-to-avalonia-migration/references/migration-checklist.md @@ -47,7 +47,7 @@ Use this when collapsing a large multi-view derisk branch into a landable PR (se ## Phase 3 — Seams - [ ] Existing seams reused from `Src/Common/FwAvalonia/Seams/` -- [ ] Any new seam added to `references/seam-catalog.md` with purpose, +- [ ] Any new seam added to `seam-catalog.md` with purpose, rules, and pivot trigger - [ ] No region code reaches directly into PropertyTable/mediator/LCModel outside a seam @@ -125,7 +125,7 @@ Use this when collapsing a large multi-view derisk branch into a landable PR (se ## Phase 10 — Retrospective (updates this skill set) - [ ] New patterns/gotchas/pivots recorded per the protocol in - `references/lessons-learned.md` + `lessons-learned.md` - [ ] New plugins added to the canonical examples in architecture-patterns.md §5 - [ ] Stale file pointers in any fieldworks-* skill fixed diff --git a/.claude/skills/jira-bugfix/SKILL.md b/.claude/skills/jira-bugfix/SKILL.md index daa0de36d3..7ca016a80f 100644 --- a/.claude/skills/jira-bugfix/SKILL.md +++ b/.claude/skills/jira-bugfix/SKILL.md @@ -1,4 +1,3 @@ -```skill --- name: jira-bugfix description: > @@ -10,425 +9,105 @@ license: MIT compatibility: Requires atlassian-skills, atlassian-readonly-skills. metadata: author: FieldWorks team - version: "1.0" + version: "2.0" --- -# JIRA Bugfix Workflow +# JIRA Bugfix -End-to-end skill for fixing bugs sourced from SIL's JIRA -(LT-prefixed tickets). Orchestrates the full lifecycle from -issue triage through PR creation and JIRA update. +Fixing a bug sourced from an LT ticket, from triage to PR. -## When to Use +Tell the user the steps before starting, then work them in order. Pause at the +four decision points below; everything else proceeds. -Activate this skill when: -- The user says "fix LT-XXXXX" or "work on LT-XXXXX" -- The user references a JIRA bug they want resolved -- You are starting a bugfix session for a known defect +## Steps -## Announce the Plan +| # | Step | Notes | +| --- | --- | --- | +| 0 | Fetch the issue | Summary, status, components, comments. Comments often carry the repro | +| 1 | Assign, move to In Progress | Skip if already there | +| 2 | Branch | `LT-XXXXX-short-slug` off fresh `origin/main` | +| 3 | **Reproduce with a failing test** | The most important step. See below | +| 4 | Fix | The minimal change that makes the test pass | +| 5 | Widen coverage | Edge cases, other call sites of the changed code, backward compatibility | +| 6 | Devil's advocate | Challenge the fix and the tests before anyone else does | +| 7 | AGENTS.md | Update only if a contract or behaviour documented there changed | +| 8 | Commit | `commit-messages` skill. gitlint runs in CI | +| 9 | PR | `pr-preflight` -- the required entrypoint. Never hand-write a PR body | +| 10 | Comment on the ticket | Root cause, fix, PR link, tests added | -Before starting any work, **always tell the user** the steps -you will follow, in order: +**Arriving from `jira-issue`?** If that skill just filed the ticket and the +user chose "start now", steps 0-2 are already done -- the issue is fetched, +assigned, In Progress, and the branch and worktree are named in a ticket +comment. Begin at step 3. -> I will follow the JIRA bugfix workflow: -> -> 1. Fetch the issue details from JIRA -> 2. Ensure the issue is assigned to you and in progress -> 3. Verify or create a branch named after the ticket -> 4. Reproduce the bug with a failing test (TDD) -> 5. Implement the minimal fix to make the test pass -> 6. Assess whether additional test coverage is needed -> 7. Devil's advocate & code review -> 8. Check and update relevant AGENTS.md files -> 9. Commit, push, and create a PR -> 10. Update the JIRA ticket with a comment and PR link -> -> I will pause for your input at key decision points. +## Step 3 -- TDD, and the escape hatch -Then proceed through the steps below. - -## Step 0: Fetch Issue from JIRA - -Use the `atlassian-readonly-skills` scripts to get the issue: - -```powershell -python -c "import sys; sys.path.insert(0, '.claude/skills/atlassian-readonly-skills/scripts'); from jira_issues import jira_get_issue; print(jira_get_issue('LT-XXXXX'))" -``` - -Extract and present to the user: -- **Summary** and **description** -- **Status**, **priority**, **assignee** -- **Components** and **affected versions** -- **Comments** (recent ones may contain reproduction steps) - -If the issue is not a Bug type, note this and ask the user -whether to proceed with the bugfix workflow anyway. - -## Step 1: Assign and Transition to In Progress - -### Check current assignment - -If the issue is unassigned or assigned to someone else, ask -the user for their JIRA username and assign it: - -```powershell -# Assign the issue (Data Center uses 'name' field) -python -c " -import sys, json -sys.path.insert(0, '.claude/skills/atlassian-skills/scripts') -from jira_issues import jira_update_issue -print(jira_update_issue('LT-XXXXX', assignee='username')) -" -``` - -> **Note**: SIL JIRA is Data Center, so the assignee field -> uses `name` (username string), not `accountId`. If the -> `jira_update_issue` call fails with assignee, fall back to -> setting it via `custom_fields`: -> `custom_fields={'assignee': {'name': 'username'}}` - -### Transition to In Progress - -```powershell -# Get available transitions -python -c " -import sys -sys.path.insert(0, '.claude/skills/atlassian-readonly-skills/scripts') -from jira_workflow import jira_get_transitions -print(jira_get_transitions('LT-XXXXX')) -" - -# Transition (use the ID for "In Progress" from above) -python -c " -import sys -sys.path.insert(0, '.claude/skills/atlassian-skills/scripts') -from jira_workflow import jira_transition_issue -print(jira_transition_issue('LT-XXXXX', 'TRANSITION_ID')) -" -``` - -If the issue is already In Progress, skip this step. - -## Step 2: Branch Management - -### Check current branch - -```powershell -git branch --show-current -``` - -**Decision tree:** - -1. If the current branch name contains the LT number - (e.g., `LT-22427`), you are already on the right branch. - Proceed. - -2. If not, **ask the user**: - - > The current branch is ``. This doesn't match - > LT-XXXXX. Options: - > - Create a new branch `LT-XXXXX` from `main` - > - Continue on the current branch - > - Switch to an existing branch (specify name) - - If creating a new branch: - ```powershell - git fetch origin - git checkout -b LT-XXXXX origin/main - ``` - -> **Important**: Do NOT create worktrees automatically. This -> repo uses worktrees but creating them involves workspace -> setup scripts. If a worktree is needed, tell the user to -> run the "Worktree: Create/Open from branch" VS Code task. - -## Step 3: Reproduce the Bug (TDD) - -**This is the most important step.** Default to test-driven -development: write a failing test that captures the bug -before writing any fix. - -### Process - -1. **Analyze the bug**: From the JIRA description and code - exploration, understand the root cause. - -2. **Find the right test file**: Locate existing tests for - the affected component. Follow the conventions in - `.github/instructions/testing.instructions.md`. - -3. **Write a failing test** that demonstrates the bug: - - Name it descriptively: - `MethodName_Scenario_ExpectedBehavior` - - The test should FAIL with the current code - - The test should PASS after the fix - -4. **Run the test** to confirm it fails: - ```powershell - .\test.ps1 -TestFilter "Name~TestMethodName" - ``` - -5. **If a test is impossible**, explain to the user WHY: - - The bug is purely visual/UI and untestable in NUnit - - The bug requires external services not available in - test harness - - The bug is in build/packaging infrastructure - - Then ask the user: - > I cannot write an automated test for this bug because - > [reason]. Would you like me to proceed with the fix - > anyway, or would you prefer to explore alternative - > verification approaches? - - Wait for confirmation before proceeding. - -## Step 4: Implement the Fix - -1. Apply the **minimal change** needed to make the failing - test pass. -2. Follow repo conventions: - - `.github/instructions/managed.instructions.md` for C# - - `.github/instructions/native.instructions.md` for C++ - - `.github/instructions/testing.instructions.md` for tests -3. Run the previously-failing test to confirm it passes: - ```powershell - .\test.ps1 -TestFilter "Name~TestMethodName" - ``` - -## Step 5: Assess Additional Test Coverage - -After the fix passes, evaluate whether additional tests are -needed. Ask yourself: - -- **Are there related edge cases** the fix might affect? -- **Are there other code paths** that use the same logic? -- **Is the existing test coverage** for this component - adequate? -- **Could a devil's advocate** argue the fix is incomplete? - -If gaps exist, add tests. Common high-value additions: -- Backward compatibility tests (old behavior still works) -- Isolation tests (fix doesn't leak to unrelated paths) -- Boundary/edge-case tests -- Tests for other call sites of modified code - -Run the full component tests: -```powershell -.\test.ps1 -TestProject "path/to/TestProject" -``` - -## Step 6: Devil's Advocate & Code Review - -Before finalizing, critically review your own work. Play -devil's advocate against the fix and the tests. - -### Challenge the fix - -Ask yourself and present findings to the user: - -- **Is this the best solution?** Are there simpler or more - robust alternatives? If multiple reasonable approaches - exist and the best choice is unclear, **present the - options to the user** with trade-offs and ask which they - prefer. -- **Does the fix introduce new risks?** Could it regress - other behavior, cause performance issues, or break - backward compatibility? -- **Is the scope right?** Is the fix too narrow (misses - related cases) or too broad (changes more than needed)? -- **Are there subtle edge cases** not covered by the tests? -- **Does the fix match the codebase style** and patterns? - Check naming, error handling, null safety, threading. - -### Clean code review - -Review the diff as if you were a code reviewer: - -- No dead code, commented-out code, or debug artifacts -- No unintended whitespace or formatting changes -- Method/variable names are clear and consistent -- Comments explain *why*, not *what* -- No accidental scope expansion beyond the bug fix -- Error paths are handled correctly - -### Decision point - -If the review surfaces **any uncertainties**, present them -to the user: - -> **Devil's advocate findings:** -> -> 1. [Finding and why it matters] -> 2. [Alternative approach and trade-offs] -> -> Would you like me to adjust the approach, or proceed -> as-is? - -Wait for the user's response before continuing. If no -uncertainties exist, briefly summarize why the fix is -solid and proceed. - -## Step 7: Update AGENTS.md - -Check whether the fix changes any contracts, behaviors, or -architecture documented in AGENTS.md files: - -1. Find relevant AGENTS.md files: - ```powershell - # Check for AGENTS.md in affected directories - Get-ChildItem -Path "Src/" -Filter "AGENTS.md" -Recurse - ``` - -2. If behavior or contracts changed, update the AGENTS.md - to reflect the new state. - -3. If no AGENTS.md exists for the affected area and the - change is architecturally significant, note this but - do not create one unless the user requests it. - -## Step 8: Commit and Push - -### Pre-commit checks +Write a failing test that captures the bug before writing any fix. Name it +`MethodName_Scenario_ExpectedBehavior`. Confirm it fails, then fix, then +confirm it passes. ```powershell -.\build.ps1 -.\test.ps1 -.\Build\Agent\check-and-fix-whitespace.ps1 +.\test.ps1 -TestFilter "Name~TestMethodName" -CommentHygiene ``` -### Commit +A filter that matches nothing still exits 0 and prints PASS. Check +`Total tests: N` is greater than zero. -Follow `.github/instructions/commit-messages.instructions.md`: -- Subject: max 72 characters, imperative mood, no trailing - punctuation -- Body: wrap at 80 characters, explain what and why +If a test is genuinely impossible -- purely visual, needs an external service, +or lives in build/packaging -- say **which** of those it is, then ask whether +to proceed without one. Do not skip silently, and do not invent a test that +passes either way. -Pattern: -``` -Fix LT-XXXXX: - - - - - -``` - -### Push - -```powershell -git add -A -git commit -m "" -git push -u origin LT-XXXXX -``` - -If the branch already exists on the remote, just `git push`. +## Step 6 -- Devil's advocate -## Step 9: Create a Pull Request +Before finalising, argue against your own work: -Use the GitHub MCP tools (or the `mcp_github_create_pull_request` -tool) to create a PR: +- Is there a simpler or more robust fix? If several are reasonable, present + them with trade-offs and let the user choose. +- Does it regress anything, or change performance or compatibility? +- Is the scope right -- not so narrow it misses sibling cases, not so broad it + becomes a refactor? +- Does the diff carry dead code, debug artefacts, or stray whitespace? -- **Title**: `Fix LT-XXXXX: ` -- **Base**: `main` (or the appropriate target branch) -- **Body** should include: - - Problem description (from JIRA) - - Root cause analysis - - Fix description with rationale - - Files changed table - - All tests added with descriptions - - Test results summary - - Design rationale for non-obvious choices +Surface any uncertainty to the user rather than resolving it silently. -Present the PR URL to the user. +## Branching and worktrees -## Step 10: Update JIRA Ticket +`LT-XXXXX-short-slug`, for example `LT-22715-nc-delete-warning`. The number +keeps step 2's "contains the LT number" check working and makes the branch +greppable; the slug is what makes a list of a dozen worktrees readable. -Add a comment to the JIRA ticket with the fix summary and -PR link: +Do not create a worktree without asking -- and do not refuse to create one +either. `scripts/Worktree-CreateFromBranch.ps1` and the `Worktree:` VS Code +tasks exist. If `.claude/.jira-issue-prefs.json` records a `workspace` +preference, follow it and say so. The script places worktrees under +`../.worktrees/` while the ones on disk are under `.tmp/worktrees/` -- +match what is already there. -```powershell -python -c " -import sys -sys.path.insert(0, '.claude/skills/atlassian-skills/scripts') -from jira_issues import jira_add_comment -comment = '''Fix implemented and PR created. - -*Root cause*: +## JIRA mechanics -*Fix*: +All of it -- create, comment, attach, link, transition, and the `custom_fields` +back doors for assignee and Affects Version -- is in +`.claude/skills/jira-issue/references/publish.md`. Do not duplicate it here. -*PR*: https://github.com/sillsdev/FieldWorks/pull/NNN +**Never transition to Done or Resolved.** That follows a merged PR, and it is +not this skill's call. -*Tests added*: -- : -- : -''' -print(jira_add_comment('LT-XXXXX', comment)) -" -``` +## Pause for the user -Do NOT transition the ticket to "Done" or "Resolved" — that -happens after the PR is merged and verified. +1. The current branch does not match the ticket. +2. A failing test cannot be written. +3. Devil's advocate surfaced a real alternative or uncertainty. +4. The fix reveals a larger problem than the ticket describes. -## IDE-Specific Notes +## When things fail -### VS Code -- Use the terminal for all Python/git/build commands -- Use the GitHub MCP tools for PR creation -- Use the `mcp_github_*` tools for PR review operations +- **JIRA unreachable** -- ask for the details, carry on, update the ticket at the end. +- **Tests fail after the fix** -- show the failures and ask; do not tune the test to pass. +- **Push rejected** -- pull with rebase, resolve, retry. -### Visual Studio -- Use the Package Manager Console or Developer PowerShell - for Python/git commands -- The JIRA Python scripts work the same way -- For PR creation, use the GitHub CLI (`gh pr create`) or - the VS GitHub extension +## Composes with -## Decision Points (Pause for User) - -This workflow pauses for user input at these points: -1. **Branch**: If current branch doesn't match the ticket -2. **Untestable bug**: If a failing test cannot be written -3. **Devil's advocate**: If the review surfaces - uncertainties or alternative approaches -4. **Scope expansion**: If the fix reveals larger issues - -All other steps proceed automatically. - -## Error Handling - -- **JIRA unreachable**: Ask user for issue details manually, - continue with the workflow, and update JIRA at the end -- **Tests fail after fix**: Present failures, ask user - whether to investigate or revert -- **Build fails**: Present errors, attempt to fix, or ask - user for guidance -- **Push rejected**: Pull with rebase, resolve conflicts, - retry - -## Integration with Other Skills - -This skill composes with: -- `atlassian-readonly-skills` — reading JIRA issues -- `atlassian-skills` — writing to JIRA (assign, comment) -- `session-workflow` — session management and handoff -- `execute-implement` — implementation conventions -- `verify-test` — test verification -- `rubric-verify` — weighted rubric scoring + hard-gate checks -- `review` — self-review before PR - -## Quick Reference - -``` -Skill chain: - jira-bugfix - ├── atlassian-readonly-skills (Step 0: read) - ├── atlassian-skills (Steps 1, 10: write) - ├── execute-implement (Step 4: fix) - ├── verify-test (Steps 3, 5: test) - ├── review (Step 6: devil's advocate) - ├── commit-messages.instructions (Step 8: commit) - └── mcp_github / gh CLI (Step 9: PR) -``` -``` +`jira-issue` (files the ticket, hands off here at step 3) · +`atlassian-skills` (JIRA writes) · `commit-messages` (step 8) · +`pr-preflight` (step 9) · `fieldworks-test-coverage` (step 5) diff --git a/.claude/skills/jira-issue/SKILL.md b/.claude/skills/jira-issue/SKILL.md new file mode 100644 index 0000000000..e1b1b0604a --- /dev/null +++ b/.claude/skills/jira-issue/SKILL.md @@ -0,0 +1,97 @@ +--- +name: jira-issue +description: "Write and file an LT Jira issue -- bug, feature or task -- that a triager can act on from the first line. Use whenever asked to file, raise, report or create a Jira issue or LT ticket, to turn a user report or a finding into a ticket, or to restructure a ticket that buries its point. Interviews, hunts duplicates before drafting, gets a three-line lede approved, and moves the analysis to a comment." +argument-hint: "Optional: the raw report, a finding, or an existing LT-XXXXX to restructure" +user-invocable: true +--- + +# Jira Issue + +Jira Data Center has no `{expand}`. Nothing folds away, so length in the +description is length on the screen for every reader, permanently. Keep the +description short and put the depth in the first comment. + +Style contract: `.claude/references/compact-style.md`. Read it first. +Screenshots: `.claude/references/evidence.md`, before publishing any image. + +## Phases + +| # | Phase | Rule | +| --- | --- | --- | +| 0 | Type | Bug, feature or task. **More than one problem means more than one ticket** | +| 0b | Relevance | Not every ticket is about FLEx. Drop sections that do not apply; never fill them with "N/A" | +| 1 | Interview | Who / when / where / how / how bad. One at a time, **max 6**. "I don't know" is recorded, not guessed | +| 2 | Duplicates | Search **before** drafting. Show at most 5 candidates as a table with verdicts | +| 3 | Lede | Three labelled lines, **approved before anything else is written**. Max 3 revisions, then ask which line is wrong | +| 4 | Body | Track file, plus the budgets in `references/format.md` | +| 5 | Permission | "Do you have permission to post this?" Hard stop. A screenshot of a live project counts | +| 6 | Publish | `references/publish.md` | +| 7 | Report | Key, URL, one `Next:` line. Nothing else | +| 8 | Start now? | Assign, transition, comment the branch and worktree, hand to `jira-bugfix` at its Step 3 | + +## The two gates + +Nothing is filed until the developer has answered both: + +1. the duplicate table (Phase 2) +2. the three lede lines (Phase 3) + +If either can be skipped quietly, the skill is decoration. + +## Non-negotiable + +- **Affects Version `FW 9.3` on every new ticket.** A filing convention, not a + claim about a build, so Phase 0b does not exempt tooling or docs tickets. +- **Nothing above the fold that the reporter did not say or you did not + verify.** Inference goes in the comment, labelled inferred. +- Budgets: summary 80 characters, lede 60 words, description 250 words ending + in one `*Next:*` line. Overflow goes to the comment. + +## Lede labels + +| Type | Labels | +| --- | --- | +| Bug | `*What happens:*` `*Who hits it:*` `*How bad:*` | +| Feature | `*What they want:*` `*Who wants it:*` `*Why it matters:*` | +| Task | `*What this is:*` `*Who it affects:*` `*Why it matters:*` | + +## Traps that have already bitten + +- **Read link types; never guess.** There is no `Relates` here. Falling back to + the first name in the list once produced four bogus `Cloners` links. +- **`resolution` cannot be set by an update** -- only by a transition. +- **The read-only skill's `jira_workflow.py` and `jira_projects.py` raise + `NameError: Optional` on import.** Use the `atlassian-skills` copies. +- **A private URL is broken evidence.** A Gmail or Drive link renders for + nobody. Attach the file. +- **Rewriting a ticket: post the original as a comment first**, then replace + the description. + +## Preferences + +`.claude/.jira-issue-prefs.json` (gitignored): `jiraUsername`, `workspace` +(`worktree` or `branch`), `branchStyle`. Ask the two questions once on first +run, then act on them silently -- but say which preference was used, so a +wrong one is visible. + +## References + +| File | For | +| --- | --- | +| `references/bug-track.md` | Interview, repro rules, sample-project permission | +| `references/duplicates.md` | The four search passes and the candidate table | +| `references/feature-track.md` | User story, workarounds, cost shape | +| `references/format.md` | Budgets, required fields, Jira markup | +| `references/publish.md` | The API calls and their gotchas | +| `references/examples.md` | LT-22715 before and after | + +## Before finishing + +- [ ] Both gates answered by the developer. +- [ ] Affects Version set. +- [ ] Description 250 words or fewer, ending in one `*Next:*` line. +- [ ] Every unknown is a `*Not known:*` line rather than a guess. +- [ ] No section emitted that does not apply. +- [ ] Every image trimmed, captioned, and labelled headless / live / mockup. +- [ ] The title and the last line alone tell the reader what is wrong and what + happens next. diff --git a/.claude/skills/jira-issue/references/bug-track.md b/.claude/skills/jira-issue/references/bug-track.md new file mode 100644 index 0000000000..bb66fe55d1 --- /dev/null +++ b/.claude/skills/jira-issue/references/bug-track.md @@ -0,0 +1,91 @@ +# Bug track + +What a FLEx bug report needs, from a real reporter's list: a brief description +of the problem, a sample project, and the steps to reproduce. Everything below +serves those three. + +## The interview + +Six questions maximum, one at a time. Stop early if the answers are already in +the raw report. + +**Who** -- which user, what role (linguist, consultant, translation advisor), +and how many people are affected. "One user" and "every project on the team" +are different tickets. + +**When** -- FLEx version *and* build number, the date it happened, whether it +is the first time or recurring, and whether it worked before. Version is +usually the decisive fact: a guard that shipped in 9.2.5 changes the whole +diagnosis depending on which side of it the reporter is on. + +**Where** -- the tool and window, the exact menu path, and which project. Ask +whether the project can be shared before assuming it can. + +**How** -- the exact actions. For text-entry bugs this is where the answer +hides: typed directly, pasted, dragged, typed with a vernacular keyboard or +IME, or arrived through Send/Receive. "Just typed it" and "pasted it" are +different bugs with different fixes. + +**How bad** -- data loss, a workaround, or blocked work. Silent data loss +outranks a visible error. Say whether Undo recovers it. + +## Steps to reproduce + +Numbered, one bounded action per step, no step containing "and then" twice. +Each step is something the reader can do without knowing the codebase. + +``` +# Open Grammar > Natural Classes. +# Create a natural class from phonological features and name it. +# Insert that class into a phonological rule formula. +# Return to Natural Classes and delete the class. +# Open the rule again. +``` + +Rules: + +- Start from a state the reader can reach: a new project, or a named sample. +- Never start at "with the corrupted project open". +- If a step needs specific data, say exactly what data. +- If reproduction is unreliable, say how many attempts out of how many. An + intermittent bug reported as reliable wastes the first hour of the fix. +- If nobody has reproduced it, say so in one line and put the inferred path in + the comment. A ticket that claims a reproduction it does not have is worse + than one that admits the gap. + +## Expected and Actual + +One line each, both observable. "It should work" is neither. + +## Environment + +FLEx version and build, Windows version, and anything unusual: Send/Receive in +use, a non-default keyboard or IME, a project migrated from an older version. + +## Sample project + +**Ask before attaching. Always.** + +> Do you have permission to post this? + +FLEx projects contain unpublished lexical data, vernacular text, and often +speaker names. The reporter may not own the data, and a Jira attachment is a +publication to everyone with project access. + +If the answer is no, or unclear: + +- Describe the shape of the data instead: how many entries, which writing + systems, which fields populated. +- Ask whether a minimal synthetic project reproduces it. +- Record in the ticket that a sample exists but was not attached, so nobody + re-asks. + +Never attach a project the agent found on disk. Never attach a screenshot of a +live project without the same question. + +## Priority + +Do not set a priority number. Give the triager the facts that determine one: +whether data is lost, whether Undo recovers it, whether a workaround exists, +and how many users are affected. Those four lines are worth more than a guess +at a field value, and the lede already carries them. diff --git a/.claude/skills/jira-issue/references/duplicates.md b/.claude/skills/jira-issue/references/duplicates.md new file mode 100644 index 0000000000..73ca30af0d --- /dev/null +++ b/.claude/skills/jira-issue/references/duplicates.md @@ -0,0 +1,36 @@ +# Duplicate and related search (Phase 2) + +**Search before drafting, not before posting.** If the ticket exists, the work +is a comment on it, and finding that out after twenty minutes wastes the +twenty minutes. + +```powershell +python -c @' +import sys; sys.path.insert(0, ".claude/skills/atlassian-skills/scripts") +from jira_search import jira_search +jql = "project = LT AND text ~ \"natural class\" AND status != Closed ORDER BY updated DESC" +print(jira_search(jql, fields="key,summary,status,updated", limit=10)) +'@ +``` + +Four passes, ten results each: + +1. **Symptom words** -- the reporter's vocabulary, not ours. +2. **Area** -- `project = LT AND component = "..." AND status != Closed`. +3. **Link walk** -- for every ticket already cited, read its links, follow one hop. +4. **Mechanism** -- the type or method name, when the code location is known. + +`text ~` searches summary, description, comments and environment, so a common +word like "triage" returns hundreds of irrelevant hits. Say so when reporting a +noisy pass rather than listing its results. + +Present at most five candidates: + +| Key | Summary | Why it might be the same | Verdict | +| --- | --- | --- | --- | + +Verdicts are duplicate, related or unrelated. **Never file without showing this +table and getting a yes.** If a duplicate exists, offer to comment on it +instead; if the developer still wants a new ticket, file it and link it. + +Link types are in `publish.md` -- read them, never guess. diff --git a/.claude/skills/jira-issue/references/examples.md b/.claude/skills/jira-issue/references/examples.md new file mode 100644 index 0000000000..863d7cf1b3 --- /dev/null +++ b/.claude/skills/jira-issue/references/examples.md @@ -0,0 +1,97 @@ +# Worked example: LT-22715 + +A real ticket, filed before this skill existed. The analysis in it is good. +The shape is the failure. + +## Before + +Summary: `No distinction between user-created and auto-generated natural classes` + +Description opened with: + +``` +h3. The underlying problem + +FLEx has two kinds of feature-based natural class and no way to tell them apart. +``` + +and ran past a thousand words through `h3. Symptoms`, `h3. Why the LT-22576 +approach cannot be extended`, `h3. Proposed signal: presence of an +Abbreviation`, a five-column table, `h3. Open challenges`, and `h3. Three ways +to resolve them`. + +Four things went wrong: + +1. **The first rendered line is a heading**, so the first thing a triager reads + is the word "problem" and nothing else. +2. **It contains four problems.** The description says so outright: "that + single missing distinction produces four separate user-visible problems." + None of the four can be triaged, prioritised or closed on its own. +3. **Analysis sits above the fold.** The comparison with LT-22576, the + proposed signal, and the three resolution options are all real and useful, + and none of them is what a triager needs in ten seconds. +4. **The blocking question is buried.** Four open challenges needing a team + decision appear after roughly eight hundred words. + +## After + +Summary: `Natural Classes: generated classes are indistinguishable from real ones` + +``` +*What they want:* A rule should show the natural class the user picked, not a +stack of features, and the Natural Classes list should not fill with entries +nobody created. +*Who wants it:* Anyone building phonological rules from features. Surfaced by +LT-22576. +*Why it matters:* Editing a shared class from inside one rule silently changes +every other rule using it. + +h3. The cause + +FLEx has two kinds of feature-based natural class and nothing in the model +separates them: ones created deliberately in Grammar > Natural Classes, and +ones fabricated silently when features are inserted into a rule. Both are +{{PhNCFeatures}}. + +h3. Symptoms, filed separately + +# LT-22724 -- editing a class from one rule changes every rule using it +# LT-22725 -- a named feature class renders as a feature list inside rules +# LT-22726 -- generated classes accumulate and are never cleaned up +# LT-22727 -- no way to promote a generated feature bundle into a real class + +h3. Ideas to resolve + +# Treat a filled Abbreviation as "this is a real class" -- one branch in +{{RuleFormulaVcBase.Display}}, no model change +# Migrate, filling Abbreviation from Name -- needs a liblcm release and a +package bump, so no longer FieldWorks-only +# Stop naming generated classes at all -- no migration, but legacy data stays +ambiguous + +h3. Open question + +Existing real classes with no Abbreviation become indistinguishable from +generated ones. A team decision is needed before any of the three are built. + +_Trade-offs, the LT-22576 comparison and the migration detail are in the first +comment._ + +*Next:* team decision on the migration question. +``` + +271 words with the Environment line. The four symptom tickets were filed on +2026-08-21 as LT-22724 through LT-22727, linked to this one with "Issue split", +and each is triageable on its own. + +## What moved to the comment + +Everything cut is still on the ticket, one scroll down: + +- Why the LT-22576 display-name heuristic cannot be extended +- The five-column rendering table +- Full detail on all three options +- All four open challenges, not just the blocking one +- The implementation note about the existing label-rendering path + +Nothing was lost. It stopped being the first thing a triager reads. diff --git a/.claude/skills/jira-issue/references/feature-track.md b/.claude/skills/jira-issue/references/feature-track.md new file mode 100644 index 0000000000..b865506947 --- /dev/null +++ b/.claude/skills/jira-issue/references/feature-track.md @@ -0,0 +1,79 @@ +# Feature track + +What a FLEx feature request needs: the user story -- what does the user want to +do -- what has been tried including workarounds, and ideas to resolve it. + +## The interview + +**Who wants it, and what are they actually trying to accomplish.** Users +request a widget when they have a goal. "Add a button that clears generated +natural classes" is a widget; "stop my rule list filling with entries I never +created" is the goal. File the goal, mention the widget as an idea. A ticket +filed as a widget forecloses every better solution. + +**How often it comes up.** Once, in one project, is a different ticket from +every workshop for three years. + +**What they do today.** The workaround is the single most useful thing in a +feature request. It shows the shape of the gap, proves the need is real, and +sometimes turns out to be good enough with one small change. + +**What "done" looks like.** In the user's terms, not ours. This is what the +reporter will check when a build ships. + +## Description skeleton + +``` +*What they want:* +*Who wants it:* +*Why it matters:* + +h3. What they do today + + +h3. Ideas to resolve +# -- +# -- + +h3. Open questions + + +*Not known:* + +_Detail in the first comment._ + +*Next:* +``` + +## Ideas to resolve + +Cap at five, ranked, one line each. Each idea carries a **cost shape**, not +hours: + +- "One branch in one file." +- "Needs a liblcm release and a package bump, so no longer a FieldWorks-only + change." +- "New model field, so a data migration." + +Cost shape is what lets a triager sequence the work. Hours are a commitment +nobody in the conversation is authorised to make. + +Detail belongs in the comment. The description gets the one-liners. + +## Open questions + +If the feature needs a decision that is not the implementer's to make -- what +happens to existing data, whether a field becomes mandatory, whether a +migration runs once or repeatedly -- say so, in one line each, under a heading +that says a team decision is needed. Burying a blocking question inside a +paragraph of analysis is how a ticket sits untouched for a year. + +## Scope + +**One ticket, one problem**, and features break this more often than bugs. A +request that reads "and while we are there we should also" is two tickets. + +When one underlying cause produces several user-visible problems, file the +problems separately and link them to one ticket describing the cause. Each +problem can then be triaged, prioritised and fixed on its own, which is the +whole point of separating them. diff --git a/.claude/skills/jira-issue/references/format.md b/.claude/skills/jira-issue/references/format.md new file mode 100644 index 0000000000..0d970bee07 --- /dev/null +++ b/.claude/skills/jira-issue/references/format.md @@ -0,0 +1,128 @@ +# Field contract, budgets, and Jira markup + +## Budgets + +| Field | Budget | Why | +| --- | --- | --- | +| Summary | 80 characters | Truncates in queue views past roughly that | +| Lede | 60 words, three labelled lines | One glance, no scrolling | +| Description | 250 words total, ending in one `*Next:*` line | Nothing can be collapsed | +| Analysis comment | As long as the reasoning deserves | Nobody is forced to scroll past it | + +Required fields, regardless of subject: + +| Field | Value | +| --- | --- | +| Affects Version (`versions`) | `FW 9.3` on every new ticket, plus any specific point release the reporter names | +| Assignee | Only when someone is taking it now. Data Center wants `{"name": ""}` | + +Count words before publishing. Over budget means the overflow was comment +material, not that the budget was wrong. + +## Summary + +Shape: `Area: what goes wrong`. + +- Names the **symptom**, not the cause. The cause is often wrong at filing + time; the symptom is what a duplicate search will match. +- No ticket references, no "Bug:", no trailing punctuation. +- Uses the reporter's vocabulary so the next person searching finds it. + +Good: `Natural Classes: deleting a feature class empties rules that use it` +Bad: `PhNCFeatures.DeletionTextTSS override suppresses the delete warning` +Bad: `Issue with natural classes` + +## Description skeleton + +``` +*What happens:* +*Who hits it:* +*How bad:* + +h3. Steps to reproduce +# +# + +h3. Expected + + +h3. Actual + + +h3. Environment +FLEx , . + +*Not known:* + +_Analysis in the first comment._ + +*Next:* +``` + +Feature track replaces Steps/Expected/Actual with the sections in +`feature-track.md`. Everything else is identical. + +This is a skeleton, not a form. Drop any heading that does not apply. + +## Relevance (Phase 0b) + +Not every LT ticket is about FLEx the product. Decide the subject before the +interview, because it decides which questions are worth one of your six. + +| Subject | Environment questions that apply | +| --- | --- | +| FLEx product | Version and build, OS, project, menu path, keyboard/IME | +| Developer tooling, agent skills, docs | Repo, branch, which skill or script. No FLEx version | +| Build, CI, installer | Branch, runner, toolchain, which script. No project | + +A template dutifully filled with "N/A" is worse than a short ticket: same scan, +no information. Drop the heading instead. + +One exception, and it is a field rather than a section: **Affects Version is +always set**, whatever the subject. + +## Jira Data Center wiki markup + +There is **no `{expand}`**. It is a Confluence macro. Do not write one, and do +not plan a description around content being hidden. + +| Need | Markup | +| --- | --- | +| Heading | `h3. Text` | +| Bold | `*text*` | +| Italic | `_text_` | +| Monospace | `{{text}}` | +| Numbered list | `# item` | +| Bullet list | `* item` | +| Code block | `{code:java}...{code}` or `{noformat}...{noformat}` | +| Quote block | `{quote}...{quote}` | +| Table | `||head||head||` then `|cell|cell|` | +| Link | `[text|https://example.com]` | +| Ticket reference | `LT-12345` -- linkifies automatically | +| Attached image | `!name.png!` or `!name.png\|thumbnail!` | + +Two traps: + +- `*` at the start of a line is a bullet, not bold. The lede labels work + because `*What happens:*` is followed by text on the same line. +- Underscores inside identifiers turn on italics. Wrap any identifier in + `{{...}}`. + +## What goes in the analysis comment + +Everything true and useful that a triager does not need in the first ten +seconds: + +- Root-cause analysis and the code path +- Evidence, probes, test output +- Inferred mechanism, labelled inferred +- Options considered and their trade-offs +- Cost shape for each option +- Anything second-hand, labelled second-hand + +Open the comment with one line saying what it is, so a reader scrolling past +knows whether to stop: + +``` +h3. Analysis (inferred unless marked verified) +``` diff --git a/.claude/skills/jira-issue/references/publish.md b/.claude/skills/jira-issue/references/publish.md new file mode 100644 index 0000000000..fe348ec5a7 --- /dev/null +++ b/.claude/skills/jira-issue/references/publish.md @@ -0,0 +1,113 @@ +# Publishing: the calls, and what bites + +Order: issue, comment, attachments, links. Write description and comment to +files first; never inline multi-line Jira markup into a command. + +## Create + +```powershell +python -c @' +import sys; sys.path.insert(0, ".claude/skills/atlassian-skills/scripts") +from jira_issues import jira_create_issue +desc = open("desc.txt", encoding="utf-8").read() +print(jira_create_issue("LT", "", "Bug", description=desc, + custom_fields={"versions": [{"name": "FW 9.3"}]})) +'@ +``` + +Then `jira_add_comment(key, comment)`, then attachments once Phase 5 is +answered, then one link per related ticket. + +## Attachments + +```powershell +python -c @' +import sys; sys.path.insert(0, ".claude/skills/atlassian-skills/scripts") +from jira_attachments import jira_add_attachment +print(jira_add_attachment("LT-XXXXX", ["01-before.png", "02-after.png"])) +'@ +``` + +Reference them with `!01-before.png!`, or `!01-before.png|thumbnail!` to keep a +long description scannable. Images belong in the analysis comment unless the +picture *is* the report. Trimming, captioning and provenance labelling are in +`.claude/references/evidence.md`; they are not optional. + +## Fields that need the custom_fields back door + +`jira_create_issue` and `jira_update_issue` do not expose these, and their +docstrings describe Jira Cloud rather than SIL's Data Center. + +| Field | Pass | +| --- | --- | +| Affects Version | `custom_fields={"versions": [{"name": "FW 9.3"}]}` | +| Assignee | `custom_fields={"assignee": {"name": "John_Lambert"}}` -- a username, **not** an accountId | +| Resolution | **Not settable by update at all.** It is not on the edit screen; only a transition sets it | + +Usernames are not email addresses. Read the caller's own with +`client.get(client.api_path("myself"))["name"]`. + +## Link types -- read them, never guess + +```powershell +python -c @' +import sys; sys.path.insert(0, ".claude/skills/atlassian-skills/scripts") +from jira_links import jira_get_link_types +print(jira_get_link_types()) +'@ +``` + +**There is no `Relates` in this Jira.** As of 2026-08-21 the types are +`Cloners, Depends on, Duplicate, Issue split, partially implements, Redesign, +Related, Requires, Solution, Story/Task, Test`. + +- Splitting one ticket into several -> **`Issue split`** +- Merely related -> **`Related`** +- Same defect -> **`Duplicate`** + +**Never fall back to the first name in the list.** Doing that once produced +four `Cloners` links between a cause ticket and its children, which reads as a +claim nobody made. If the intended type is absent, stop and ask. + +Then `jira_create_issue_link(link_type, inward_issue_key, outward_issue_key)`. + +## Broken scripts to route around + +`.claude/skills/atlassian-readonly-skills/scripts/jira_workflow.py` and +`jira_projects.py` both raise `NameError: name 'Optional' is not defined` on +import -- a missing typing import. The `atlassian-skills` copies work. Use +those, for reads as well as writes, until the read-only ones are fixed. + +## Rewriting an existing ticket + +**Post the original as a comment before replacing the description.** Someone +has already read that text and may have replied to it; replacing it outright +destroys the record. + +``` +h3. Full detail (original description, preserved ) + +The description above was shortened so a triager can act on the first line. +Nothing was deleted -- the original text follows verbatim. + +---- +``` + +Then update the description. Every edit notifies watchers, so a bulk pass is a +mail burst: do it in one sitting and tell the team it is coming. + +## Phase 8 -- starting work + +1. Branch `LT-XXXXX-short-slug` off fresh `origin/main`. +2. Workspace per the saved preference. Say which one was used. +3. Comment on the ticket naming both, so the ticket indexes the worktree list: + + > Taken. Working on branch `LT-22715-nc-delete-warning`, worktree + > `.tmp/worktrees/nc-delete-warning`. + +4. Hand to `jira-bugfix` at its Step 3. Its Steps 0-2 are already done. + +Resolve transitions by ID from `jira_get_transitions`, never by guessing a +name. A permissions failure degrades to "assigned, please move it to In +Progress yourself" rather than aborting. **Never transition to Done or +Resolved** -- that follows a merged PR. diff --git a/.claude/skills/pr-pitch/SKILL.md b/.claude/skills/pr-pitch/SKILL.md index 961e2e66f2..94905a0401 100644 --- a/.claude/skills/pr-pitch/SKILL.md +++ b/.claude/skills/pr-pitch/SKILL.md @@ -6,314 +6,135 @@ argument-hint: "Optional PR number (defaults to the PR for the current branch)" # PR Pitch -**Not an entrypoint.** `pr-preflight` is the single entrypoint for making a -PR; it calls this skill for the write-up step. Invoke this directly only to -redo the write-up on a PR that already exists. +**Not an entrypoint.** `pr-preflight` calls this for the write-up. Invoke it +directly only to redo the write-up on an existing PR. -Related skills this one calls: `fieldworks-code-commenting` (when doc text -is inlined into source), `fieldworks-migration-scope-review` (when the PR is -a large migration branch and the pitch must justify its scope). +Read `.claude/references/compact-style.md` before writing, and +`.claude/references/evidence.md` before publishing any screenshot. ## What this produces -Two artifacts, always together, never one without the other: +Two artifacts, always together: -1. **The PR body**, in two zones: - - **The pitch** -- 200-400 words, above the fold, uncollapsed. - - **The accordions** -- collapsed `
` sections below it, holding - the decisions, evidence, and paths not taken. -2. **A commit** that deletes the provenance sources from the branch, so - research and working notes inform the reviewer without merging. +1. **The PR body** -- a pitch of 200-400 words above the fold, then collapsed + `
` accordions holding decisions, evidence and paths not taken. +2. **A commit deleting the provenance sources** from the branch, so research + and working notes inform the reviewer without merging. -**Everything goes in the body -- never a separate comment.** The PR -description is the one place a reader always looks, it is inherently sticky -(editing it is always in place, the URL never changes), and it is the only -part of the PR that survives being read a year later without scrolling a -thread. A second comment splits the record for no gain: nothing here needs a -comment's affordances, and no CI job posts alongside it. +Do both or neither. Accordions without the deletion merges the scaffolding; +deletion without accordions loses the reasoning. -If you write the accordions but skip the deletion, the branch merges its own -scaffolding. If you delete without the accordions, the reasoning is lost. Do -both or do neither. +**Everything goes in the body, never a separate comment.** The description is +the one place a reader always looks, editing is in place, and it survives being +read a year later without scrolling a thread. ## The rule that drives everything -A reviewer arrives at the PR with the **same unknowns the author started -with**, and no time to rediscover them. The pitch closes that gap; the -accordions hold what a future maintainer -- not a reviewer -- will want when -they ask "why is it like this?" a year from now. +A reviewer arrives with the **same unknowns the author started with**, and no +time to rediscover them. So, for every sentence: -So the test for every sentence: - -- A reviewer needs it to say yes or no today -> **the pitch**. -- A reviewer would want it only to *check* a claim the pitch makes -> - **an accordion**. State the claim above the fold, put the proof below it. - This is where most over-long bodies go wrong: the proof is real and - interesting and belongs on the PR, just not above the fold. -- Nobody needs it today, but someone will in a year -> **an accordion**. -- Neither -> **delete it**. +| Test | Zone | +| --- | --- | +| A reviewer needs it to say yes or no today | The pitch | +| They would want it only to *check* a claim the pitch makes | An accordion | +| Nobody needs it today, but someone will in a year | An accordion | +| Neither | Delete it | ## Phase 1 -- Triage every markdown file on the branch -List what the branch adds or changes: - ``` git diff --name-status main...HEAD -- '*.md' ``` -Classify each file into exactly one bucket. Judge by what the file **is**, -not where it lives. +Judge by what the file **is**, not where it lives. | Bucket | Test | Destination | | --- | --- | --- | -| DURABLE | Someone changing this code next year must read it to change it correctly | Stays in repo -- align it in Phase 2 | -| RESEARCH | A one-time investigation, audit, census, or survey whose conclusion is now baked into the code | Provenance comment, then delete | -| NOT-TAKEN | Options considered and rejected, alternatives, tradeoffs, abandoned approaches | Provenance comment, then delete | -| PROCESS | Task checklists, burn-down guides, review checklists, working notes tied to *doing* the work | Provenance comment (only the durable findings), then delete | -| STALE | Describes code that no longer exists | Delete. Salvage to provenance only if it records a real reversal | - -Two traps: - -- **A spec is not automatically durable.** A spec that merely narrates what - the code now plainly says is RESEARCH. Durable means it carries - constraints the code cannot express: invariants, contracts with other - subsystems, rejected designs that will otherwise be re-proposed. -- **Skills are usually durable, references are usually not.** A - `SKILL.md` that guides future work stays. A `references/*.md` capturing - one migration's findings is RESEARCH. - -Present the triage to the developer as a table and get confirmation before -deleting anything. Deletion is theirs to approve. +| DURABLE | Someone changing this code next year must read it to change it correctly | Stays -- align it in Phase 2 | +| RESEARCH | A one-time investigation whose conclusion is now baked into the code | Accordion, then delete | +| NOT-TAKEN | Options considered and rejected | Accordion, then delete | +| PROCESS | Checklists and working notes tied to *doing* the work | Accordion (durable findings only), then delete | +| STALE | Describes code that no longer exists | Delete. Salvage only a real reversal | -## Phase 2 -- Align what stays with the code as it is - -**Always trust the written code.** When a doc and the code disagree, the -code is right and the doc is wrong -- never the reverse, and never -"reconcile" by softening the doc into vagueness. - -For each DURABLE file, verify every concrete noun it names -- type, -interface, method, namespace, file path, setting key, test name -- against -the tree: - -``` -git grep -l "" -- 'Src/*' -``` +Two traps: **a spec is not automatically durable** -- one that narrates what +the code plainly says is RESEARCH; durable means constraints the code cannot +express. And **skills are usually durable, references usually not.** -Zero hits means the doc is wrong. Find what the thing is called now and fix -the doc. Renames, folder moves, and split interfaces are the usual cause. +Present the triage as a table and get confirmation. **Deletion is the +developer's to approve.** -Also re-check every **count** a doc asserts ("~8 call sites", "12 markers"). -Counts rot silently. Recount from the tree and correct. +## Phase 2 -- Align what stays -Do not rewrite prose that is still accurate. This phase is a correction -pass, not a rewrite. +**Always trust the written code.** Where a doc and the code disagree, the doc +is wrong -- never the reverse, and never "reconcile" by going vague. -## Phase 3 -- Write the pitch (the top zone) +Verify every concrete noun a DURABLE file names -- type, method, path, setting +key, test name -- with `git grep -l "" -- 'Src/*'`. Zero hits means the +doc is wrong; find the current name. Recount every **count** it asserts; counts +rot silently. This is a correction pass, not a rewrite. -**Budget: 200-400 words, and it must fit on one screen without scrolling.** -That is the binding constraint, not a target. Reviewers skim the description -and leave for the diff; a body that runs past a screen buries the one -paragraph that would have saved them an hour, and the length itself reads as -"the author could not say what this does." +## Phase 3 -- The pitch -Scale by risk, not by diff size. An 82,000-line branch whose whole story is -"one flag, defaulted off" gets a *shorter* body than a 200-line change to a -payment path, because there is less a reviewer must hold in their head. +**200-400 words, fitting one screen without scrolling.** That is binding, not +a target. Scale by *risk*, not diff size: an 82,000-line branch whose story is +"one flag, defaulted off" gets a shorter body than a 200-line payment change. -The budget applies to the top zone only. The accordions below it are as long -as the branch's reasoning deserves -- a closed `
` costs a reader -nothing. There is no tension between "short" and "complete" here; there is -only the question of which zone a sentence belongs in. +When a section will not fit, it was accordion material -- move it down and +leave one line up top. Never compress by deleting the qualifiers that make a +claim honest. -A reviewer arrives with two kinds of gap. **Known unknowns** -- the questions -they already know they have on opening this diff -- and **unknown unknowns**, -the things only you can see because you built it. The top zone answers those -two and nothing else. Everything you happen to know that answers neither goes -below the fold, or nowhere. +In order: -The overflow rule: when a section will not fit the budget, that section was -accordion material. Move it down and leave one line up top. Never compress by -deleting the qualifiers that make a claim honest. +**0. Status** (optional, one line) -- only for a PR open a while. What it is +waiting on, and anything red. *"Ready for review. CI green except the +known-flaky interlinear test."* -The top zone, in this order. +**1. What it does** (2-3 sentences) -- the concrete thing, not the framing. +Never open with "This PR refactors". If the change is visible, **a picture is +expected, not optional**: trimmed, captioned with what to look at, and +labelled headless / live / mockup. The image supports the claim; the test pins +it. -### 1. Lead with what it does (2-3 sentences) +**2. The unknown they start with** (one paragraph) -- usually "what breaks?" +or "why is this so big?". Answer it, then reframe the review around the +question worth their time. -Open with the concrete thing, not the framing. A screenshot or GIF if the -change is visible; otherwise one sentence naming what a user or caller can -now do that they could not before. Never open with "This PR refactors...". +**3. Where to look** (at most five bullets) -- open with one +`**Start here:**` line naming the first file to read and why. That is the +*entry point*, not the riskiest thing; a reviewer not told where to begin +reads the diff alphabetically, which is nobody's reading order. Then the +failure points a domain expert would anticipate, ordered by what would sink +the PR -- one line each: the risk, and the thing that pins it. **The proof goes +in an accordion**; inlining it is what blows the budget. -### 2. The unknown the reviewer starts with (one paragraph) +**4. What is deliberately not here** -- deferrals, parity gaps, narrower +paths. A reviewer who finds an unlisted gap stops trusting the whole pitch. +Name the marker or follow-up PR, not the reasoning. -State the question the reviewer will actually have on opening a diff this -size -- usually "what breaks?" or "why is this so big?" -- answer it, and -then reframe the review around the question that *is* worth their time. Do -not make them read to find it. +**5. Stack and verification** -- what it merges into and in what order. Build, +tests, manual checks, and what was *not* run. Anything red goes here in plain +words; a reviewer must never learn of a red job from the checks tab. Give +anything they must run themselves as numbered, copy-pasteable steps -- prose +verification instructions get skipped. -### 3. Where to look (at most five bullets, one line each) +**6. What you want from them** (one line) -- *"Next: approve, or tell me to +split the installer change out."* A pitch ending on a verification paragraph +leaves the reviewer guessing whether they are approving, splitting or blocking. -The load-bearing section. The failure points a domain expert would -anticipate, ordered by what would sink the PR -- not by what was hardest to -build. One line each: the risk, and the thing that pins it -- the gate, the -test, the invariant. +### Pitch rules -The evidence for each line goes in an accordion below. A reviewer who wants -to check rather than trust opens it; one who trusts the summary never pays -for it. Do not inline the proof here -- that is what blew the budget on every -over-long body this skill exists to prevent. - -### 4. What is deliberately not here (one list, one line each) - -Deferrals, parity gaps, known-narrower paths. A reviewer who finds an -unlisted gap stops trusting the whole pitch, so listing them buys more than -it costs. Name the in-code marker or the follow-up PR, not the reasoning -- -reasoning goes in an accordion. - -### 5. Stack and verification (a few lines) - -If the PR is stacked, say what it merges into and in what order. Then build, -tests, manual checks -- and what was *not* run. Anything currently red or -known-broken goes here in plain words; a reviewer must never learn of a red -job from the checks tab after reading a body that implied green. - -Pitch rules: - -- No process narration. "We then discovered...", "after several - iterations..." -- cut. The reviewer is approving a result, not a journey. +- No process narration. "We then discovered", "after several iterations" -- cut. - No apology, no hedging, no "should be fine". -- No section that exists only to demonstrate rigor. Depth belongs below the - fold, where it costs the reviewer nothing. +- No section that exists only to demonstrate rigor. - Every claim with a name in it must be true of the current tree. Re-verify - claims carried over from an earlier version of the body; long-lived PRs - accumulate stale ones. -- Word-count the result before publishing. Over 400, cut -- do not rationalize. - -## Phase 4 -- Write the accordions (the bottom zone) + claims carried over from an earlier version of the body. +- Word-count before publishing. Over 400, cut -- do not rationalize. -**Synthesize; do not paste.** Dumping the specs into `
` blocks is -the failure mode this skill exists to prevent. The accordions carry what -someone will care about later, in your words, with the reasoning intact and -the scaffolding gone. +## Phases 4 and 5 -They go directly below the pitch in the same PR body, after a `---` rule. -Open with a short orienting accordion that says what this record is and why -it exists outside the tree -- a reader a year out needs to know the working -documents were deliberately deleted, not lost. - -```markdown ---- - -
-Reading this a year from now -- start here - -What this record is, and why the reasoning lives here instead of in the -tree. - -
- -
-Decisions, and why - -... - -
-``` - -Sections worth writing, when the branch has them: - -- **Reading this a year from now** -- the orienting preamble. First. -- **The layer cake** -- for a branch that introduces an architecture, the - path from input to output with the real type at each hop. This is the - single most useful thing for someone arriving cold. -- **Decisions, and why** -- the choice, the alternatives, what tipped it. - Prefer decisions where the code looks arbitrary until you know the reason. -- **Paths not taken** -- what was tried or seriously considered and - rejected, and the reason. The single most valuable section; it is what - stops the next person re-proposing a dead end. -- **Reversals** -- things built and then removed, with why. Note which ones - are invisible in `git log` because they happened inside a squash. -- **Surprising findings** -- what the investigation turned up that - contradicted the initial assumption. -- **What this does NOT authorize** -- for a foundational branch, the limits. - A later reader will otherwise cite the branch as precedent for more than - it decided. -- **Deferred, and what would unblock it** -- what was scoped out and what - would need to be true to pick it up. -- **Evidence** -- the proof behind the pitch's "Where to look" bullets: the - predicate quoted, the call-site count, the equivalence argument, the test - that pins it. Written for a reviewer rather than a maintainer, so it can - sit last; the pitch already carries the claim. - -Rules: - -- Each section stands alone. Nobody reads these top to bottom. -- Attribute nothing to a person; describe the decision, not the deciders. -- GitHub caps a PR body at 65,536 characters. If you are near it, you are - pasting, not synthesizing. Cut harder. -- Reasoning recoverable *only* from git history -- because the branch - deleted the document that argued it -- is the highest-value content here. - Prefer it over anything a reader could derive by reading the tree. - -## Phase 5 -- Apply - -In this order: - -1. Delete the RESEARCH / NOT-TAKEN / PROCESS / STALE files (`git rm`), after - the developer has confirmed the Phase 1 triage. -2. Commit the Phase 2 alignment edits and the deletions together, with a - message saying the reasoning moved to the PR. -3. Push. -4. Update the PR body -- pitch and accordions, one write. - -Write the body to a file first -- never pass it inline. Then: - -```powershell -# gh pr edit is fine when the token has the scope; when it does not -# (it wants org:read), go straight to REST. -gh pr edit --body-file body.md -gh api -X PATCH repos///pulls/ -F body=@body.md --jq '.body|length' -``` - -The body is inherently sticky: editing it is always in place and the PR URL -never changes, so nothing needs marker-matching or an existence check. That -is the main reason the record lives here rather than in a comment. - -If an earlier run of this skill left a separate provenance comment, fold its -content into the accordions and delete it, so there is exactly one record: - -```powershell -gh api -X DELETE repos///issues/comments/ -``` - -Deleting is destructive and public -- confirm with the developer first, and -only after its content is verifiably in the published body. - -Anything in the tree that pointed at that comment now dangles. Check before -you delete, and repoint at the PR description: - -``` -git grep -n "provenance comment\|issuecomment" -- openspec/ Docs/ Src/ -``` +Accordions: `references/accordions.md`. Applying and the final checklist: +`references/publishing.md`. -Note that a wrapped line will defeat a naive grep for a two-word phrase -- -search for each word. - -Before finishing, confirm: - -- [ ] The pitch zone is 200-400 words and fits one screen. Count, do not - estimate. -- [ ] Every `
` is closed -- count `
` against `
`. -- [ ] The whole body is under 65,536 characters. -- [ ] No content lives in a PR comment; the description is the only record. -- [ ] Nothing in the tree references a comment that was deleted. -- [ ] No deleted file's content was lost -- each is represented in an - accordion or was deliberately dropped as STALE. -- [ ] Every name in the body resolves in the current tree -- accordions rot - the same way the pitch does, and a rename sweep late in a branch will - have stranded names the earlier reasoning used. -- [ ] Every count was recounted. -- [ ] The pitch does not repeat what an accordion already says. -- [ ] Working notes are gitignored (`Docs/migration/working/`), not merged. - -Do not mark this complete on unverified claims. If a claim could not be -checked, say so in the report rather than asserting it. +Related skills this calls: `fieldworks-code-commenting` (when doc text is +inlined into source), `fieldworks-migration-scope-review` (large migration +branches). diff --git a/.claude/skills/pr-pitch/references/accordions.md b/.claude/skills/pr-pitch/references/accordions.md new file mode 100644 index 0000000000..13af123331 --- /dev/null +++ b/.claude/skills/pr-pitch/references/accordions.md @@ -0,0 +1,43 @@ +# The accordions (the bottom zone) + +Everything below the pitch, in collapsed `
` blocks after a `---`. +A closed `
` costs a reader nothing, so length here is free -- but +**synthesise, do not paste.** Dumping the branch's specs into `
` +blocks is the failure mode this skill exists to prevent. + +Open with a short orienting block, so a reader a year out knows the working +documents were deliberately deleted rather than lost. + +```markdown +--- + +
+Reading this a year from now -- start here + +What this record is, and why the reasoning lives here instead of in the tree. + +
+``` + +## Sections worth writing, when the branch has them + +| Section | Carries | +| --- | --- | +| **Reading this a year from now** | The orienting preamble. Always first | +| **The layer cake** | For a branch introducing an architecture: input to output, with the real type at each hop. The most useful thing for someone arriving cold | +| **Decisions, and why** | The choice, the alternatives, what tipped it. Prefer decisions where the code looks arbitrary until you know the reason | +| **Paths not taken** | What was tried or seriously considered and rejected. The highest-value section -- it stops the next person re-proposing a dead end | +| **Reversals** | Built then removed, and why. Note which are invisible in `git log` because they happened inside a squash | +| **Surprising findings** | What contradicted the initial assumption | +| **What this does NOT authorize** | For a foundational branch, the limits. Otherwise it gets cited as precedent for more than it decided | +| **Deferred, and what would unblock it** | Scoped out, and what would need to be true to pick it up | +| **Evidence** | The proof behind the pitch's "Where to look" bullets. Written for a reviewer rather than a maintainer, so it sits last | + +## Rules + +- Each section stands alone. Nobody reads these top to bottom. +- Attribute nothing to a person. Describe the decision, not the deciders. +- GitHub caps a body at 65,536 characters. Near it means you are pasting. +- **Reasoning recoverable only from git history** -- because the branch + deleted the document that argued it -- is the highest-value content here. + Prefer it over anything a reader could derive by reading the tree. diff --git a/.claude/skills/pr-pitch/references/publishing.md b/.claude/skills/pr-pitch/references/publishing.md new file mode 100644 index 0000000000..71d83bfa6b --- /dev/null +++ b/.claude/skills/pr-pitch/references/publishing.md @@ -0,0 +1,67 @@ +# Applying the pitch + +Order: + +1. `git rm` the RESEARCH / NOT-TAKEN / PROCESS / STALE files, **after** the + developer has confirmed the triage. +2. Commit the alignment edits and the deletions together, with a message + saying the reasoning moved to the PR. +3. Push. +4. Update the PR body -- pitch and accordions, one write. + +Write the body to a file first. Never pass it inline. + +```powershell +gh pr edit --body-file body.md +``` + +`gh pr edit` needs `read:org` on the token and will fail with a GraphQL scope +error without it. Go straight to REST when it does: + +```powershell +gh api -X PATCH repos///pulls/ -F body=@body.md --jq '.body|length' +``` + +The body is sticky: editing is in place and the PR URL never changes, so +nothing needs marker-matching or an existence check. That is the main reason +the record lives here rather than in a comment. + +## Folding in an old provenance comment + +If an earlier run left a separate comment, move its content into the +accordions and delete it, so there is exactly one record: + +```powershell +gh api -X DELETE repos///issues/comments/ +``` + +Deleting is destructive and public -- confirm first, and only once the content +is verifiably in the published body. Anything in the tree pointing at that +comment now dangles: + +``` +git grep -n "provenance comment\|issuecomment" -- openspec/ Docs/ Src/ +``` + +A wrapped line defeats a naive two-word grep. Search for each word. + +## Before finishing + +- [ ] The pitch zone is 200-400 words and fits one screen. Count, do not estimate. +- [ ] It opens with `**Start here:**` and closes with a `Next:` line. +- [ ] Every `
` is closed -- count `
` against `
`. +- [ ] Whole body under 65,536 characters. +- [ ] No content lives in a PR comment; the description is the only record. +- [ ] Nothing in the tree references a comment that was deleted. +- [ ] No deleted file's content was lost -- each is in an accordion, or was + deliberately dropped as STALE. +- [ ] Every name in the body resolves in the current tree. Accordions rot the + same way the pitch does, and a late rename sweep strands names the + earlier reasoning used. +- [ ] Every count was recounted. +- [ ] The pitch does not repeat what an accordion already says. +- [ ] Working notes are gitignored (`Docs/migration/working/`), not merged. +- [ ] The pre-send check in `.claude/references/compact-style.md` passes. + +Do not mark this complete on unverified claims. If a claim could not be +checked, say so in the report rather than asserting it. diff --git a/.claude/skills/pr-preflight/SKILL.md b/.claude/skills/pr-preflight/SKILL.md index f032ea6f1d..098acf72d1 100644 --- a/.claude/skills/pr-preflight/SKILL.md +++ b/.claude/skills/pr-preflight/SKILL.md @@ -7,269 +7,112 @@ user-invocable: true # PR Preflight -Use this skill when the user wants an interactive FieldWorks branch review before posting or updating a PR. +Interactive branch review before a PR is posted or updated. This is the +orchestration layer: review policy lives in +`.github/instructions/review-analyzer.instructions.md`, shared terminology in +`CONTEXT.md`, and the write-up belongs to `pr-pitch`. -This is the orchestration layer. The review policy lives in `.github/instructions/review-analyzer.instructions.md`, shared terminology lives in `CONTEXT.md`, and specialized reviewer agents may be used for independent read-only passes. - -## Goals - -- Analyze the branch diff from `origin/main` using FieldWorks review policy. -- Use specialist agents for deep read-only review where they fit the changed files. -- Challenge the author on risks, assumptions, validation gaps, and design understanding. -- Record author explanations, dismissed findings, unresolved concerns, and in-review fixes. -- Write a fresh `.review/summary.md` that a reviewer can use as a meeting agenda. -- Optionally commit, push, and create or update a PR only after the author confirms readiness, delegating the write-up to `pr-pitch`. - -## Start Here - -First tell the author what will happen: - -> **Here's what this preflight will do:** -> -> 1. **Setup** - Check your branch and ask a couple of quick questions -> 2. **Analysis** - Review contracts/correctness, managed UI/localization, native interop/COM, build/test/installer risk, and validation evidence -> 3. **Interview** - Walk through findings and challenge the reasoning -> 4. **Output** - Write `.review/summary.md` and optionally create or update a PR -> -> During the interview, you can explain changes, dismiss findings with reasons, ask me to fix something, or say you are unsure. I will record all of that for the reviewer. +Tell the author what will happen before starting: setup, analysis, interview, +then `.review/summary.md` and optionally the PR. During the interview they can +explain, dismiss a finding with a reason, ask for a fix, or say they are +unsure -- all of which get recorded. ## Setup -1. Determine the review model name. Use `GitHub Copilot` when running in Copilot. Do not invent AI co-author trailers. -2. Validate the current branch: - - Run `git branch --show-current`. - - If the branch is `main`, stop and tell the author to run this from a feature branch. -3. Check working tree state with `git status --porcelain`. - - If there are uncommitted changes, explain that any fixes made during the review will be staged and may be committed with existing changes at the end. - - Ask whether the author wants to commit existing work first or continue with it included. -4. Compute the diff range against `origin/main`. - - Run `git fetch origin --quiet`. - - Compute `MERGE_BASE` with `git merge-base origin/main HEAD`. - - Compute `HEAD_SHA`, `FILE_COUNT`, and `INITIAL_COMMIT_COUNT`. - - List changed files with `git diff --name-only MERGE_BASE`. - - If this fails, ask the author which base branch to use. -5. Check whether `.review` is ignored. - - If `.review` is missing from `.gitignore`, ask whether to add it now. - - Do not combine this question with the purpose question. -6. Determine the branch purpose. - - Use any purpose supplied in the prompt invocation. - - If none was supplied, ask: "What is the overall purpose of these changes? Please describe it in your own words." - -## Context And Language Check - -Before analysis, load `CONTEXT.md` and `.github/context/codebase.context.md`. - -If the branch purpose, PR title, plan, or spec uses overloaded FieldWorks terms such as `project`, `model`, `view`, `app`, `context`, `review`, or `validation`, apply the `grill-with-docs` discipline before writing the summary: +1. `git branch --show-current`. **Stop if it is `main`.** +2. `git status --porcelain`. If dirty, ask whether to commit first; in-review + fixes will otherwise be committed alongside. +3. `git fetch origin --quiet`, then merge-base against `origin/main`. Record + the file count and commit count. +4. If `.review` is not gitignored, ask to add it -- as its own question. +5. Ask the branch purpose in the author's own words, unless supplied. -- Clarify the term with the author. -- Ground the term in code, docs, tests, or build files. -- Update `CONTEXT.md` only for durable shared language decisions. -- Carry the clarified terms into findings, interview notes, and PR copy. +Load `CONTEXT.md` and `.github/context/codebase.context.md`. If the purpose or +title uses an overloaded FieldWorks term -- `project`, `model`, `view`, `app`, +`context`, `review`, `validation` -- apply `grill-with-docs` before writing +the summary, and carry the clarified term into the findings and PR copy. ## Analysis -Load `.github/instructions/review-analyzer.instructions.md` and run all four required passes: - -- Contracts, compatibility, and correctness. -- Managed UI, C#, and localization. -- Native, COM, and boundary safety. -- Build, tests, CI, dependencies, and installer. - -Use specialist agents as independent read-only reviewers when the changed files justify them and the agent tooling is available. Keep them scoped; the final synthesis remains your responsibility. - -Recommended agents: - -- `FieldWorks C# Expert` for managed `*.cs`, `.csproj`, config, resources, or net48 behavior. -- `FieldWorks WinForms Expert` for WinForms UI, designer, layout, event-handler, resource, or localization changes. -- `FieldWorks C++ Expert` for native, C++/CLI-adjacent, COM, Views, FwKernel, ViewsInterfaces, or ABI-sensitive changes. -- `FieldWorks Avalonia UI Expert` only for Avalonia/XAML work; do not use it for existing WinForms UI. -- `devils-advocate` for large architecture, scope, or risk arguments where a skeptical pass would sharpen the interview. - -If specialist agents are unavailable or would add friction for a small diff, run the passes directly using the review policy. - -For each pass: - -- Compare against `MERGE_BASE`. -- Verify findings against actual code before reporting. -- Record findings as Critical, Important, or Minor. -- Record positive observations. -- Record required validation and evidence gaps. - -After all passes: - -- Merge findings into one list ordered by severity. -- Deduplicate only when two passes flagged the same file for the same concern. -- Keep distinct concerns about the same file as separate findings. -- Keep a factual Contract/API Changes summary. -- Keep Required Validation separate from findings. - -## Author Interview - -Aim for 5-15 questions total. Ask one Critical or Important finding at a time unless multiple findings share one root cause. - -For each Critical and Important finding: - -- Ask directly why the change is safe or intentional. -- Ask what validation covers it. -- If the answer is vague, ask one follow-up. -- If it remains unclear after the follow-up, record it as unresolved. - -If the changes are large, cross native/managed/build/installer boundaries, involve non-obvious design decisions, or the author's answers reveal uncertainty, ask separately: - -> "Can you walk me through the most complex or non-obvious part of these changes? I want to make sure I understand the reasoning." - -Watch for lack-of-understanding signals: - -- "The AI did it" or similar deferrals. -- "I'm not sure" or "I don't know". -- Vague explanations that do not describe the mechanism. -- Inability to explain a changed section. - -Record those explicitly as `Author does not understand: `. Do not soften them into acceptance or satisfaction. - -For Minor findings: - -- Print the full list first. -- If there are 3 or fewer, ask whether the author wants to respond to all at once or one at a time. -- If there are more than 3, go one at a time. - -End the interview by asking: - -> "Is there anything else you want to flag or discuss before I write up the summary? Any tradeoffs you made, things you're uncertain about, or context a reviewer should know?" - -## In-Review Fixes - -If the author asks you to fix a finding, implement the fix unless it is ambiguous. Ask one clarifying question only when needed. - -Rules: - -- Keep fixes minimal and scoped to the finding. -- Stage fixes with `git add`; do not commit yet. -- Record each fix as `INTERVIEW_CHANGES` with the finding and what changed. -- Do not remove fixed findings from the summary; mark them `[x]` with a fixed-during-review note. - -After in-review fixes, run relevant FieldWorks checks: - -- Use repository scripts and tasks, not ad-hoc `msbuild`, `dotnet build`, `vstest.console`, or `nmake`. -- Prefer the VS Code task `CI: Whitespace check` for whitespace. -- Run `./build.ps1` when build-affecting, managed, native, resource, or project files changed. -- Run `./test.ps1` with the narrowest reliable `-TestProject` or `-TestFilter` for managed behavior. -- Run `./test.ps1 -Native` with `-TestProject` when native code or tests changed. -- Run `./Build/Agent/Setup-InstallerBuild.ps1 -ValidateOnly` for installer/WiX/helper-script changes. -- Do not mark manual validation complete unless you directly performed it or the author explicitly confirms it. - -Report checks that were skipped and why. - -## Summary File - -Always write a fresh `.review/summary.md`. Do not merge with an existing summary. - -Use this structure: - -```markdown -# Code Review Summary - -**Branch**: - -**Base**: - -**Date**: - -**Review model**: - -**Files changed**: - -## Overview - -[One or two paragraphs combining the author's purpose with the analysis result.] - -## Contract/API Changes - -[Factual Contract/API Changes summary. Write "None." if none.] - -## Findings - -Finding states: -- `- [ ] **Description**` - open -- `- [ ] ~~Description~~ _(author's explanation)_` - dismissed -- `- [x] **Description** _(fixed during review: what changed)_` - fixed - -### Critical - Must address before merge - -[All Critical findings, or "None."] - -### Important - Should address before merge - -[All Important findings, or "None."] - -### Minor - Consider - -[All Minor findings, or "None."] - -## Required Validation / Evidence - -[Commands run, commands still needed, manual validation gaps, or "None."] +Run all four passes from `review-analyzer.instructions.md`: -## Positive Observations +1. Contracts, compatibility, correctness +2. Managed UI, C#, localization +3. Native, COM, boundary safety +4. Build, tests, CI, dependencies, installer -[Positive observations, or "None."] +Use specialist read-only agents where the changed files justify one -- +`FieldWorks C# Expert`, `WinForms Expert`, `C++ Expert`, `Avalonia UI Expert` +(Avalonia work only), `devils-advocate` for large scope or risk arguments. Run +the passes directly for a small diff. The synthesis is yours either way. -## Interview Notes +Per pass: compare against the merge base, **verify each finding against the +actual code before reporting it**, and grade Critical / Important / Minor. +Record positive observations and validation gaps too. Merge into one severity +-ordered list; deduplicate only when two passes flagged the same file for the +same concern. -[Author explanations, decisions, unresolved items, and explicit lack-of-understanding notes.] +## Interview -## In-Review Quality Check +5-15 questions. One Critical or Important finding at a time, unless several +share a root cause. -[Only include if in-review changes were made.] +Per finding: why is this safe or intentional, and what validation covers it? +One follow-up if the answer is vague; if still unclear, record it unresolved. -## Suggested Review Focus +For large, cross-boundary or non-obvious changes, ask separately: -- [ ] [High-priority review meeting agenda item] -- [ ] [High-priority review meeting agenda item] -``` +> "Can you walk me through the most complex or non-obvious part of these +> changes? I want to make sure I understand the reasoning." -## PR Offer +**Record lack of understanding literally.** "The AI did it", "I'm not sure", +or an explanation that never describes the mechanism becomes +`Author does not understand: `. Never soften it into acceptance. -After writing the summary, tell the author: +Minor findings: print them all first. Three or fewer, ask whether to take them +together; more than three, go one at a time. -> "Review summary written to `.review/summary.md`. -> -> Please review it, make changes where appropriate, and run `/pr-preflight` again until you are ready to post the PR. -> -> If you do not want to make any changes and are ready for review, would you like me to commit any uncommitted changes, push, and post the PR? I will check whether one already exists for this branch and update it, or create a new one if not. The write-up runs through `pr-pitch`, which will also triage any research or working markdown on the branch into collapsed sections in the PR body and out of the tree -- you approve that triage before anything is deleted." +Close with: "Anything else to flag -- trade-offs, uncertainties, context a +reviewer should know?" -Only create or update a PR after the author confirms. +## In-review fixes -## PR Description +Keep them minimal and scoped to the finding. `git add`, do not commit yet. +Record each as `INTERVIEW_CHANGES`. **Do not delete a fixed finding** -- mark +it `[x]` with a fixed-during-review note. -This skill is the single entrypoint for making a PR, but it does not compose -the description itself. Once the author confirms readiness, invoke the -`pr-pitch` skill and let it own the write-up. It produces two artifacts -together: the PR body (a pitch above the fold, provenance in collapsed -accordions below it) and a commit evicting the branch's research and working -markdown from the tree. +Then run the repo scripts, never ad-hoc `msbuild` / `dotnet build` / +`vstest.console` / `nmake`: -Hand `pr-pitch` the branch purpose, the findings, and `.review/summary.md`. +| Changed | Run | +| --- | --- | +| Anything build-affecting | `./build.ps1 -CommentHygiene` | +| Managed behaviour | `./test.ps1` with the narrowest reliable `-TestProject` or `-TestFilter` | +| Native code or tests | `./test.ps1 -Native -TestProject

` | +| Installer, WiX, helper scripts | `./Build/Agent/Setup-InstallerBuild.ps1 -ValidateOnly` | +| Whitespace | VS Code task `CI: Whitespace check` | -The preflight record still belongs on the PR, but as a collapsed section -below the pitch, not as the opening content: +A `-TestFilter` that matches nothing still exits 0 and prints PASS -- check +`Total tests: N` is above zero. **Never mark manual validation complete unless +you performed it or the author explicitly confirms it.** Report skipped checks +and why. -```markdown -

-Preflight review details +## Summary and PR - -[summary] - +Write `.review/summary.md` per `references/summary-template.md`. -
-``` +Then offer -- and only act on confirmation: -Keep the markers so a re-run replaces the section instead of appending a -second copy. Do not duplicate findings, interview notes, or validation logs -into the pitch itself -- the pitch states validation status in a sentence -and leaves the detail here. +> "Summary written to `.review/summary.md`. Review it, make changes where +> appropriate, and re-run until you are ready. When you are, shall I commit, +> push and post the PR? I will update an existing one if there is one. The +> write-up runs through `pr-pitch`, which also triages the branch's research +> and working markdown into collapsed sections in the PR body and out of the +> tree -- you approve that triage before anything is deleted." -For branch names like `lt-1234-anything`, prefix the PR title with `LT-1234:`. Use a sentence-case title based on the actual change, not just the branch slug. +**This skill never composes the description itself.** Hand `pr-pitch` the +branch purpose, the findings and the summary. For a branch named +`lt-1234-anything`, prefix the PR title `LT-1234:` and write a sentence-case +title from the actual change, not the branch slug. -After Copilot or human reviewers leave comments, use `.github/prompts/respond-to-review-comments.prompt.md` to work through the review response loop: evaluate comments, fix sensible requests, ask about ambiguity, verify, commit, push, reply, resolve, and summarize. +After reviewers comment, use `respond-to-review-comments`. diff --git a/.claude/skills/pr-preflight/references/summary-template.md b/.claude/skills/pr-preflight/references/summary-template.md new file mode 100644 index 0000000000..250e6785dd --- /dev/null +++ b/.claude/skills/pr-preflight/references/summary-template.md @@ -0,0 +1,79 @@ +# `.review/summary.md` template + +Always write it fresh. Never merge into an existing summary. + +```markdown +# Code Review Summary + +**Branch**: +**Base**: +**Date**: +**Review model**: +**Files changed**: + +## Overview + +[One or two paragraphs combining the author's stated purpose with what the +analysis found.] + +## Contract/API Changes + +[Factual. "None." if none.] + +## Findings + +### Critical - Must address before merge +### Important - Should address before merge +### Minor - Consider + +## Required Validation / Evidence + +[Commands run, commands still needed, manual gaps, or "None."] + +## Positive Observations + +## Interview Notes + +[Author explanations, decisions, unresolved items, and explicit +lack-of-understanding notes.] + +## In-Review Quality Check + +[Only if in-review changes were made.] + +## Suggested Review Focus + +- [ ] [agenda item] +``` + +Finding states: + +| Markup | Means | +| --- | --- | +| `- [ ] **Description**` | Open | +| `- [ ] ~~Description~~ _(author's explanation)_` | Dismissed, with the reason | +| `- [x] **Description** _(fixed during review: what changed)_` | Fixed | + +A dismissed finding keeps its strikethrough and its reason. Deleting it hides +that the question was ever asked. + +## Carrying the summary onto the PR + +The preflight record belongs on the PR as a **collapsed section below the +pitch**, never as the opening content: + +```markdown +
+Preflight review details + + +[summary] + + +
+``` + +Keep the markers so a re-run replaces the section instead of appending a +second copy. Do not duplicate findings, interview notes or validation logs +into the pitch -- the pitch states validation status in a sentence and leaves +the detail here. diff --git a/.gitignore b/.gitignore index a4c7c89ff4..6db7bd9207 100644 --- a/.gitignore +++ b/.gitignore @@ -223,3 +223,6 @@ DataTreeTimingBaselines.json Docs/migration/working/ Build/Agent/comment-hygiene-report.json .review/ + +# Per-developer preferences for the jira-issue skill +.claude/.jira-issue-prefs.json