sillsdev / sillsdev/python-sil-lift

add_ranges_file appends a second header reference when the companion spells the range id in another normalization

Open
#33 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug
Dominant language
Python
Stars
1
Forks
0
Avg merge
10d 11h
Merged PRs (30d)
6

Description

Summary

Lexicon.add_ranges_file decides which ranges the header already references by exact string
comparison, so a header that spells a range id in NFC and a companion that spells it in NFD
are treated as unrelated ranges: a second <range> header reference is appended for the
same conceptual range, and save() writes both. FLEx mixes normalizations between an id and
the references to it (#14, #27), so a lexicon derived from a real export is exactly where
the two spellings meet.

Reproduction

import sil_lift, unicodedata

nfd = lambda s: unicodedata.normalize("NFD", s)
name = "Catégorie"

lex = sil_lift.Lexicon()
lex.header.ranges.append(sil_lift.Range(id=name, href="dup.lift-ranges"))  # NFC in the header
ranges = sil_lift.RangesFile()
ranges.add_range(nfd(name)).add_element("Nom")                             # NFD in the companion
lex.add_ranges_file(ranges, href="dup.lift-ranges")

print([ascii(r.id) for r in lex.header.ranges])
lex.save("dup.lift")
["'Cat\\xe9gorie'", "'Cate\\u0301gorie'"]

and in the saved .lift:

<ranges>
  <range id="Catégorie" href="dup.lift-ranges"/>
  <range id="Catégorie" href="dup.lift-ranges"/>
</ranges>

Two references, rendering identically, differing only by normalization — one of which the
caller never asked for.

Cause

src/sil_lift/_model.py:825-829:

referenced = {range_.id for range_ in self.header.ranges}
for range_ in ranges_file.ranges:
    if range_.id not in referenced:
        self.header.ranges.append(Range(id=range_.id, href=href))
        referenced.add(range_.id)

The membership test is exact, so the two spellings of one name are different keys.

Expected

Resolve referenced the way the validator resolves a name to an id — exact spelling first,
then NFC — and append nothing when the header already references the range under either
spelling. The existing header id must not be rewritten: whichever spelling the document came
with is the one it keeps.

Scope

Write path only. Validation of such a document is already correct as of #28, which resolves
a header range/@id against a companion's range id under NFC and reports the split as a
normalization-mismatch warning. This is about what save() then writes.

Distinct from #29: that is the merged read view (all_ranges()) dropping a range when two
companions define the same id. Same underlying theme — ids compared as exact strings — but a
different code path and a different symptom. See also the all_ranges() NFC-keying note on
that issue.

Notes

Pre-existing; not introduced by #28.

Reachability is narrow: it needs a header that already references the range under one
spelling plus a call to add_ranges_file with a companion spelling it the other way — the
documented "call again to reference ranges added later" flow on a FLEx-derived lexicon. No
corpus fixture exercises it, so this needs a hand-authored case.

One design decision comes with it: the package's only NFC machinery is three closures inside
_semantic_problems (src/sil_lift/_validate.py:432). Fixing this needs either a second
unicodedata.normalize call in _model.py or promoting nfc/resolve into a shared
private module. The latter is probably right if any other module ever needs it, but it is
worth deciding deliberately rather than inlining a call a later refactor has to undo.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start at add_ranges_file in src/sil_lift/_model.py:825-829 and compare its reference lookup with the NFC resolution closures in src/sil_lift/_validate.py:432. Add a hand-authored regression case for NFC/NFD range ids, preserving the existing header spelling while avoiding a duplicate reference. Done means save() writes one reference and validation behavior remains unchanged.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
backend
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
72/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.