Versification.load gives an error when a custom.vrs contains duplicate verse definitions.

Summary: 'Facts' by Claude, rewritten in English I think I understand.

Loading a custom versification from a stream applies the "no overriding a built-in versification" rule that should only apply to built-in versifications. As a result, a custom.vrs that defines a verse segment twice causes an error: RuntimeError: Invalid versification syntax: duplicate segment, line: N

Paratext tolerates duplicate verse definintions in the custom.vrs file, ignoring previous definitions and keeping only the last one.
FileParatextProjectSettingsParser.parse() crashes on projects with a custom.vrs that contains a duplicated verse definition. It's pretty rare though so this isn't high priority.

Observed on sil-machine 1.9.2.
Minimal reproduction:

import pathlib, tempfile
from io import BytesIO
from machine.scripture.verse_ref import Versification

# A custom.vrs that defines the same verse segment twice, as some Paratext projects do.
custom_vrs = b"*ESG 4:8,-,a\n*ESG 4:8,-,a\n"
base = Versification.get_builtin(4)  # English

# Loading by PATH succeeds (last-wins override, which is what Paratext does):
p = pathlib.Path(tempfile.mkdtemp()) / "custom.vrs"
p.write_bytes(custom_vrs)
Versification.load(p, base, "by-path")            # OK

# Loading the IDENTICAL bytes by STREAM raises:
Versification.load(BytesIO(custom_vrs), base, "by-stream")
# RuntimeError: Invalid versification syntax: duplicate segment, line: 2

In Versification._load (machine/scripture/verse_ref.py) the two branches differ only in the filename they forward to parse:

• path branch (line 719): return cls.parse(file_stream, file, versification, fallback_name)
  https://github.com/sillsdev/machine.py/blob/8dedd19d992906d3244215047aae4e4cefd3ddd8/machine/scripture/verse_ref.py#L719

• stream branch (line 726): return cls.parse(file_stream, None, versification, fallback_name)
  https://github.com/sillsdev/machine.py/blob/8dedd19d992906d3244215047aae4e4cefd3ddd8/machine/scripture/verse_ref.py#L726

• settings parser loads custom.vrs from a stream: https://github.com/sillsdev/machine.py/blob/8dedd19d992906d3244215047aae4e4cefd3ddd8/machine/corpora/paratext_project_settings_parser_base.py#L47-L48

_parse_versification derives "is this a built-in versification?" purely from that filename (line 1097):

• is_builtin = filename is None: https://github.com/sillsdev/machine.py/blob/8dedd19d992906d3244215047aae4e4cefd3ddd8/machine/scripture/verse_ref.py#L1097

_parse_verse_segments_line uses is_builtin to decide whether or not to override the check: (lines 1326–1327):
https://github.com/sillsdev/machine.py/blob/8dedd19d992906d3244215047aae4e4cefd3ddd8/machine/scripture/verse_ref.py#L1326-L1327

# Don't allow overwrites for built-in versifications
if is_builtin and bbbcccvvv in versification.verse_segments:
    raise _syntax_error("duplicate segment", parsed_line.line_num)

Claude suggested that we derive the is_builtin from the presence of a base versification or fallback_name rather than from "filename is None". I would have thought that we could base it on whether or not we are parsing a custom.vrs file. I'm not sure of the difference between those two though.

Start with Versification._load and _parse_versification in machine/scripture/verse_ref.py, comparing the path and stream branches and the duplicate check in _parse_verse_segments_line. Run the minimal reproduction and inspect FileParatextProjectSettingsParser.parse() to confirm the affected stream path. Done means a custom.vrs with duplicate definitions loads from a stream with the last definition retained, while built-in duplicate protection remains intact.