Skip to content

Commit 6532d87

Browse files
ddaspitclaude
andcommitted
Port 'Better USFM versification analysis' from machine PR #432
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 3520bdc commit 6532d87

13 files changed

Lines changed: 982 additions & 648 deletions

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

Lines changed: 145 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,145 @@
1+
---
2+
name: port-pr
3+
description: Port a machine (C#) PR into machine.py (Python). Given a GitHub issue number for a porting task in sillsdev/machine.py, finds the linked machine (C#) PR, ports its changes to the Python codebase, runs the local checks (black/flake8/isort/pyright/pytest), and opens a PR that closes the issue. Use when asked to port a PR/issue from machine (C#), complete a "porting" issue, or sync a machine change into machine.py.
4+
---
5+
6+
# Port a machine (C#) PR into machine.py (Python)
7+
8+
`machine` (C#) and `machine.py` (Python) are direct, intentionally-synced ports of each
9+
other. This skill ports a change that already landed in `machine` into `machine.py`,
10+
driven by a "porting" issue in `sillsdev/machine.py`.
11+
12+
**Required argument:** the GitHub issue number in `sillsdev/machine.py` (always exists).
13+
14+
## Repos
15+
16+
- Python target repo: the current working directory (`sillsdev/machine.py`).
17+
- C# source repo: the sibling clone at `../machine` (`sillsdev/machine`).
18+
Use the local clone for reading surrounding context; use `gh ... --repo sillsdev/machine`
19+
for authoritative PR data.
20+
21+
## Step 1 — Read the porting issue
22+
23+
```bash
24+
gh issue view <ISSUE> --json title,body,labels
25+
```
26+
27+
- The body looks like: `Port any relevant changes in https://github.com/sillsdev/machine/pull/<PR> from machine to machine.py.`
28+
Extract `<PR>` — the machine (C#) PR number — from that URL.
29+
- The issue title looks like `Port '<Title>'`. Keep `<Title>` for the branch and PR.
30+
- If the body has no machine (C#) PR link, stop and ask the user for the source PR.
31+
32+
## Step 2 — Understand the source change
33+
34+
```bash
35+
gh pr view <PR> --repo sillsdev/machine --json title,body,files,commits
36+
gh pr diff <PR> --repo sillsdev/machine
37+
```
38+
39+
Read the full diff. For each changed C# file, open the corresponding file(s) in
40+
`../machine` to understand the surrounding context, and identify the Python counterpart
41+
(see mapping below). Read the existing Python code you're about to change so the port
42+
matches local idiom.
43+
44+
Note: not every change ports. Skip C#-only concerns (`.csproj`/`.sln`/`Directory.*.props`,
45+
`AssemblyInfo`, `omnisharp.json`, csharpier/editorconfig formatting, NuGet packaging).
46+
The issue says "any *relevant* changes" — use judgment and call out anything you
47+
intentionally skip.
48+
49+
## Step 3 — File & API mapping
50+
51+
| machine (C#) | machine.py (Python) |
52+
|---|---|
53+
| `src/SIL.Machine/<PascalArea>/<PascalCase>.cs` (or the matching `SIL.Machine.*` project) | `machine/<area>/<snake_case>.py` |
54+
| `tests/SIL.Machine.Tests/<PascalArea>/<PascalCase>Tests.cs` (or the matching `*.Tests` project) | `tests/<area>/test_<snake_case>.py` |
55+
| `PascalCase` methods / `camelCase` locals / `_camelCase` fields | `snake_case` functions/vars |
56+
| `IReadOnlyList<T>` / `IDictionary<,>` / `ISet<T>` etc. | `Sequence[T]`/`list` / `Mapping`/`dict` / `Set`/`set` etc. |
57+
| NUnit `Assert.That(...)` | pytest plain `assert` (check neighboring test files) |
58+
| `AssemblyInfo`/`.csproj` `<Version>` | `pyproject.toml` `version` (poetry) |
59+
60+
The top-level Python areas are: `annotations`, `clusterers`, `corpora`, `jobs`,
61+
`optimization`, `punctuation_analysis`, `scripture`, `sequence_alignment`, `statistics`,
62+
`tokenization`, `translation`, `utils`. Some C# code lives in tool/plugin projects
63+
(`SIL.Machine.Tool`, `SIL.Machine.Translation.Thot`, `SIL.Machine.Morphology.HermitCrab`,
64+
etc.); the Python equivalent may live under `machine/jobs` or a different area, or may not
65+
exist at all. Find the Python counterpart by searching for the type/method name (translated
66+
to snake_case): `grep -ri "<type_or_method_name>" machine tests` before assuming a path.
67+
68+
Port the **behavior**, not the syntax. Match existing Python patterns in the neighboring
69+
code (naming, type hints, dataclasses, generators vs. loops, async conventions). Port the
70+
tests too.
71+
72+
## Step 4 — Branch & apply
73+
74+
Create a branch off `main` (do not commit to `main`):
75+
76+
```bash
77+
git switch main && git pull && git switch -c port-<slug>
78+
```
79+
80+
where `<slug>` is a short kebab-case form of the issue title (e.g.
81+
`port-update-library-version-to-1.8.11`).
82+
83+
Apply the ported changes with Edit/Write.
84+
85+
## Step 5 — Verify locally
86+
87+
Install, format, lint, type-check, and test (this is `local_check.sh`):
88+
89+
```bash
90+
poetry install
91+
poetry run black .
92+
poetry run flake8 .
93+
poetry run isort .
94+
poetry run pyright
95+
poetry run pytest
96+
```
97+
98+
`black` and `isort` rewrite files in place; `flake8` and `pyright` are gates that must pass
99+
clean. Fix any formatting, lint, type, or test failures before proceeding. Report the test
100+
results plainly (pass/fail counts); don't claim success if anything failed.
101+
102+
## Step 6 — Commit, push, open PR (pause first)
103+
104+
Show the user a summary of the diff and the proposed PR title/body, and **confirm before
105+
pushing**. Then:
106+
107+
```bash
108+
git add -A
109+
git commit # message: Port '<Title>' from machine PR #<PR>
110+
git push -u origin port-<slug>
111+
gh pr create --title "Port '<Title>' from machine" --body "<body>"
112+
```
113+
114+
Commit message footer:
115+
116+
```
117+
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
118+
```
119+
120+
### PR body template
121+
122+
```markdown
123+
Ports [machine PR #<PR>](https://github.com/sillsdev/machine/pull/<PR>) — <one-line summary of the change>.
124+
125+
## <Section per area changed>
126+
<What changed and why, mirroring the source PR. Include short before/after or code snippets where helpful.>
127+
128+
## Tests
129+
<Tests ported / added.>
130+
131+
Closes #<ISSUE>
132+
```
133+
134+
PR body footer:
135+
136+
```
137+
🤖 Generated with [Claude Code](https://claude.com/claude-code)
138+
```
139+
140+
## Notes
141+
142+
- Keep the two codebases as similar as is reasonable for a Python-vs-C# port.
143+
- If the source PR spans multiple commits, the squashed PR diff is the source of truth, but
144+
reading individual commits can clarify intent.
145+
- If a change has no sensible Python counterpart, say so in the PR body rather than forcing it.

machine/corpora/__init__.py

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
from .file_paratext_project_settings_parser import FileParatextProjectSettingsParser
1212
from .file_paratext_project_terms_parser import FileParatextProjectTermsParser
1313
from .file_paratext_project_text_updater import FileParatextProjectTextUpdater
14-
from .file_paratext_project_versification_error_detector import FileParatextProjectVersificationErrorDetector
14+
from .file_usfm_versification_analyzer import FileUsfmVersificationAnalyzer
1515
from .flatten import flatten
1616
from .memory_alignment_collection import MemoryAlignmentCollection
1717
from .memory_stream_container import MemoryStreamContainer
@@ -28,7 +28,6 @@
2828
from .paratext_project_settings_parser_base import ParatextProjectSettingsParserBase
2929
from .paratext_project_terms_parser_base import KeyTerm, ParatextProjectTermsParserBase
3030
from .paratext_project_text_updater_base import ParatextProjectTextUpdaterBase
31-
from .paratext_project_versification_error_detector_base import ParatextProjectVersificationErrorDetectorBase
3231
from .paratext_text_corpus import ParatextTextCorpus
3332
from .place_markers_usfm_update_block_handler import PlaceMarkersAlignmentInfo, PlaceMarkersUsfmUpdateBlockHandler
3433
from .scripture_element import ScriptureElement
@@ -78,10 +77,12 @@
7877
from .usfm_update_block import UsfmUpdateBlock
7978
from .usfm_update_block_element import UsfmUpdateBlockElement, UsfmUpdateBlockElementType
8079
from .usfm_update_block_handler import UsfmUpdateBlockHandler
81-
from .usfm_versification_error_detector import (
82-
UsfmVersificationError,
83-
UsfmVersificationErrorDetector,
84-
UsfmVersificationErrorType,
80+
from .usfm_versification_analyzer_base import UsfmVersificationAnalyzerBase
81+
from .usfm_versification_analyzer_handler import (
82+
UsfmVersificationAnalysis,
83+
UsfmVersificationAnalyzerHandler,
84+
UsfmVersificationDiagnostic,
85+
UsfmVersificationDiagnosticType,
8586
)
8687
from .usx_file_alignment_collection import UsxFileAlignmentCollection
8788
from .usx_file_alignment_corpus import UsxFileAlignmentCorpus
@@ -93,7 +94,7 @@
9394
from .zip_paratext_project_settings_parser import ZipParatextProjectSettingsParser
9495
from .zip_paratext_project_terms_parser import ZipParatextProjectTermsParser
9596
from .zip_paratext_project_text_updater import ZipParatextProjectTextUpdater
96-
from .zip_paratext_project_versification_detector import ZipParatextProjectVersificationErrorDetector
97+
from .zip_usfm_versification_analyzer import ZipUsfmVersificationAnalyzer
9798

9899
__all__ = [
99100
"AlignedWordPair",
@@ -114,7 +115,7 @@
114115
"FileParatextProjectSettingsParser",
115116
"FileParatextProjectTermsParser",
116117
"FileParatextProjectTextUpdater",
117-
"FileParatextProjectVersificationErrorDetector",
118+
"FileUsfmVersificationAnalyzer",
118119
"flatten",
119120
"is_scripture",
120121
"KeyTerm",
@@ -139,7 +140,6 @@
139140
"ParatextProjectSettingsParserBase",
140141
"ParatextProjectTermsParserBase",
141142
"ParatextProjectTextUpdaterBase",
142-
"ParatextProjectVersificationErrorDetectorBase",
143143
"ParatextTextCorpus",
144144
"parse_usfm",
145145
"PlaceMarkersAlignmentInfo",
@@ -187,9 +187,11 @@
187187
"UsfmUpdateBlockElement",
188188
"UsfmUpdateBlockElementType",
189189
"UsfmUpdateBlockHandler",
190-
"UsfmVersificationError",
191-
"UsfmVersificationErrorDetector",
192-
"UsfmVersificationErrorType",
190+
"UsfmVersificationAnalysis",
191+
"UsfmVersificationAnalyzerBase",
192+
"UsfmVersificationAnalyzerHandler",
193+
"UsfmVersificationDiagnostic",
194+
"UsfmVersificationDiagnosticType",
193195
"UsxFileAlignmentCollection",
194196
"UsxFileAlignmentCorpus",
195197
"UsxFileText",
@@ -200,5 +202,5 @@
200202
"ZipParatextProjectSettingsParser",
201203
"ZipParatextProjectTermsParser",
202204
"ZipParatextProjectTextUpdater",
203-
"ZipParatextProjectVersificationErrorDetector",
205+
"ZipUsfmVersificationAnalyzer",
204206
]

machine/corpora/file_paratext_project_versification_error_detector.py renamed to machine/corpora/file_usfm_versification_analyzer.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,10 @@
44
from .file_paratext_project_file_handler import FileParatextProjectFileHandler
55
from .file_paratext_project_settings_parser import FileParatextProjectSettingsParser
66
from .paratext_project_settings import ParatextProjectSettings
7-
from .paratext_project_versification_error_detector_base import ParatextProjectVersificationErrorDetectorBase
7+
from .usfm_versification_analyzer_base import UsfmVersificationAnalyzerBase
88

99

10-
class FileParatextProjectVersificationErrorDetector(ParatextProjectVersificationErrorDetectorBase):
10+
class FileUsfmVersificationAnalyzer(UsfmVersificationAnalyzerBase):
1111
def __init__(self, project_dir: StrPath, parent_settings: Optional[ParatextProjectSettings] = None) -> None:
1212
super().__init__(
1313
FileParatextProjectFileHandler(project_dir),

machine/corpora/paratext_project_versification_error_detector_base.py renamed to machine/corpora/usfm_versification_analyzer_base.py

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,14 @@
1-
from typing import List, Optional, Set, Union
1+
from typing import Dict, Optional, Set, Union
22

33
from ..scripture.canon import book_id_to_number
44
from .paratext_project_file_handler import ParatextProjectFileHandler
55
from .paratext_project_settings import ParatextProjectSettings
66
from .paratext_project_settings_parser_base import ParatextProjectSettingsParserBase
77
from .usfm_parser import parse_usfm
8-
from .usfm_versification_error_detector import UsfmVersificationError, UsfmVersificationErrorDetector
8+
from .usfm_versification_analyzer_handler import UsfmVersificationAnalysis, UsfmVersificationAnalyzerHandler
99

1010

11-
class ParatextProjectVersificationErrorDetectorBase:
11+
class UsfmVersificationAnalyzerBase:
1212
def __init__(
1313
self,
1414
paratext_project_file_handler: ParatextProjectFileHandler,
@@ -20,18 +20,28 @@ def __init__(
2020
else:
2121
self._settings = settings
2222

23-
def get_usfm_versification_errors(
24-
self, handler: Optional[UsfmVersificationErrorDetector] = None, books: Optional[Set[int]] = None
25-
) -> List[UsfmVersificationError]:
26-
handler = handler or UsfmVersificationErrorDetector(self._settings)
23+
def analyze_usfm_versification(
24+
self,
25+
books_and_chapters: Optional[Union[Dict[str, Optional[Set[int]]], Dict[int, Optional[Set[int]]]]] = None,
26+
handler: Optional[UsfmVersificationAnalyzerHandler] = None,
27+
) -> UsfmVersificationAnalysis:
28+
book_nums_and_chapters = (
29+
{
30+
(book_id_to_number(book) if isinstance(book, str) else book): chapters
31+
for book, chapters in books_and_chapters.items()
32+
}
33+
if books_and_chapters is not None
34+
else None
35+
)
36+
handler = handler or UsfmVersificationAnalyzerHandler(self._settings, book_nums_and_chapters)
2737
for book_id in self._settings.get_all_scripture_book_ids():
2838

2939
file_name = self._settings.get_book_file_name(book_id)
3040

3141
if not self._paratext_project_file_handler.exists(file_name):
3242
continue
3343

34-
if books is not None and not book_id_to_number(book_id) in books:
44+
if book_nums_and_chapters is not None and book_id_to_number(book_id) not in book_nums_and_chapters:
3545
continue
3646

3747
with self._paratext_project_file_handler.open(file_name) as sfm_file:
@@ -45,4 +55,4 @@ def get_usfm_versification_errors(
4555
f". Error: '{e}'"
4656
)
4757
raise RuntimeError(error_message) from e
48-
return handler.errors
58+
return handler.get_analysis()

0 commit comments

Comments
 (0)