sillsdev / sillsdev/python-sil-lift
Second companion defining the same range id is dropped, causing false undefined-range-value
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 1
- Forks
- 0
- Avg merge
- 10d 11h
- Merged PRs (30d)
- 6
Description
Summary
When two tracked .lift-ranges companions define the same range id, Lexicon.all_ranges()
keeps the first and drops the second entirely. Every range check runs over that merged
view, so the second companion's elements are never validated — and, because they are also
missing from the merged range, entry values that those elements define are reported as
undefined-range-value.
Reproduction
import sil_lift
lex = sil_lift.Lexicon()
a = sil_lift.RangesFile()
a.add_range("grammatical-info").add_element("Noun")
lex.add_ranges_file(a, href="a.lift-ranges")
b = sil_lift.RangesFile()
b.add_range("grammatical-info").add_element("Verb", parent="Nooun") # dangling parent
lex.add_ranges_file(b, href="b.lift-ranges")
entry = sil_lift.Entry(id="e1", guid="11111111-2222-4444-8888-111111111111")
entry.lexical_unit["en"] = "e1"
entry.senses.append(sil_lift.Sense(id="s1", grammatical_info=sil_lift.GrammaticalInfo("Verb")))
lex.entries.append(entry)
print([el.id for el in lex.all_ranges()["grammatical-info"].elements])
for p in lex.iter_problems():
print(p.level, p.code, "|", p.message)
['Noun']
warning undefined-range-value | grammatical-info value 'Verb' is not defined in the range
Two things are wrong there:
Verbis defined, by a companion this lexicon tracks. The warning is a false
positive against the document as a whole.parent="Nooun"matches no sibling id in any normalization, and norange-parenterror
is reported — that element was never looked at.
Cause
Lexicon.all_ranges() merges by exact id, first companion winning:
for ranges_file in self.ranges_files.values():
for range_ in ranges_file.ranges:
merged.setdefault(range_.id, range_)
_semantic_problems then iterates all_ranges.values(), so a range id that two
companions define is only ever seen in one of its definitions. Elements are not unioned:
the loser's are dropped, not added.
Still covered
Each companion is separately validated against the ranges schema, and duplicate-guid
scopes per rendered document, so both companions are scanned for guid reuse. It is only
the checks that read the merged view — range-parent, undefined-range-value,
normalization-mismatch — that see one definition.
Options
- Report it. A new warning when two tracked companions define the same range id says
plainly that one definition is being ignored. Cheapest, and honest, but leaves the
false positive above in place. - Union the elements of same-id ranges in
all_ranges(). Fixes both symptoms, but
changes documented public behavior ("the external definition (from any tracked ranges
file) is used"), and a synthesizedRangewould break callers that rely on the merged
view returning the very object a companion holds — validation does, to address a
finding to the file that defines it. - Leave
all_ranges()alone and widen validation: walk each companion's own ranges
for the structural checks, and resolve values against the union of ids. Keeps the
public contract, addresses each finding to the companion it belongs to, and costs one
more pass.
3 (optionally with 1) looks right, but the tradeoff deserves a second opinion before
anything is written.
Notes
Pre-existing; not introduced by #28, whose ranges_paths identity check is correct as
written (it maps only the range object the merged view actually holds). No corpus fixture
exercises a duplicate range id across companions — real FLEx exports write a single
companion — so this needs a hand-authored folder fixture whichever way it is fixed.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with Lexicon.all_ranges() and _semantic_problems, then reproduce the duplicate range-id case from the issue. Decide between reporting duplicates, unioning elements, or widening validation, and add a hand-authored folder fixture covering the missing range-parent and false undefined-range-value findings; done means the chosen behavior is tested without regressing companion-specific validation.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- backend
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 45/100