sillsdev / sillsdev/python-sil-lift
An entry removed, saved past, and re-added keeps a stale dateModified
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 1
- Forks
- 0
- Avg merge
- 10d 11h
- Merged PRs (30d)
- 6
Description
An entry that leaves the lexicon and comes back loses its stamping baseline, so a
later edit ships under the date the library generated earlier.
Lexicon._stamps is keyed by id(entry) and holds a strong reference to keep that
id stable. Each stamping save rebuilds the dict from the entries present, which is
what stops a removed entry's subtree from being retained — and is also why the
baseline is gone if the same object is re-added. With no baseline, _needs_stamp
reads the entry's library-generated date as caller-set and leaves it alone.
lexicon = sil_lift.load(path)
entry = sil_lift.Entry(id="temporary")
lexicon.entries.append(entry)
lexicon.save(out, when=MARCH_4) # stamped 2026-03-04, baseline recorded
lexicon.entries.remove(entry)
lexicon.save(out, when=MARCH_4) # the rebuild drops its record
lexicon.entries.append(entry)
entry.lexical_unit["en"] = "edited"
lexicon.save(out, when=MARCH_5) # keeps 2026-03-04
Scope is wider than appended entries
The trigger is not "the entry was appended". It is: the entry's _stamps record
was dropped by an intervening save, and the date it carries differs from its
parse-time record's date — or it has no parse-time record at all. Three
populations qualify.
1. Appended or from-scratch entries. No parse-time record ever exists, so
_needs_stamp takes its no-baseline branch and returns entry.date_modified is
None. The repro above.
2. Loaded entries an earlier save has stamped (or the caller has hand-dated).
at_parse.get(id(entry)) does still return a record here, so the no-baseline
branch is not the one that fires. The second branch is, and it demands both that
the digest moved and that the date still equals the baseline's. An earlier
stamped save moved the date off the parse-time date, so the second half fails and
the entry is left alone. Same outcome, different route — and it only takes one
ordinary edit-and-save beforehand:
loaded, never stamped before date after final edit: 2026-03-05... stamped=True
loaded, stamped by an earlier save date after final edit: 2026-03-04... stamped=False
Both rows are a loaded entry from a scannable document, removed, saved past,
re-added, and edited.
3. Every entry of an unscannable document. When the scan is declined
(lexicon._source is None), _parse_time_records returns {} and the reader's
_attach_stamp_baseline seeding is the only baseline there is. Dropping it leaves
nothing.
So the unusual part of the sequence is remove → save → re-add, not the entry's
provenance. A tool that filters or merges lexicons is squarely in this library's
audience.
Since note_caller_dates now drops absent records too, the intervening save
reaches this through stamp=False as well as through a stamping save.
Current behavior is pinned
tests/test_stamp.py::test_a_re_added_entry_keeps_the_date_it_left_with asserts
the stale date, so the behavior is a decision on the record rather than an
accident. Taking this up means flipping that assertion.
A weak design closes it without reintroducing the retention:
Entrygainsweakref_slot=True. Works on the leaf class alone (the slots
dataclass bases carry no__weakref__); fields, repr,==, deepcopy and pickle
are unaffected, at ~16 bytes per entry.- Stamp records hold a
weakref.ref[Entry]. Parse-time records stay strong —
removed_entries()is what returns a removed entry intact. - Every lookup checks
record.entry() is entryrather than trusting the id key.
This is what the strong reference does today, and it is not optional: id() reuse
after a free is observable within a few hundred thousandEntryallocations. - No finalizers. Each save keeps the records whose referent is still alive.
The cost is two record types where there is now one, touched by the reader, the
writer and removed_entries(); weakref_slot as permanent public surface; and
Entry alone getting it, so extending stamping to the other eight date-bearing
types would need the same again.
Worth doing if entries moving between lexicons or filtering pipelines are real
workflows; otherwise the current behavior is a consistent reading of "a dated entry
with no baseline was dated deliberately" and belongs in the docs instead. Filed for
that decision.
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 tests/test_stamp.py::test_a_re_added_entry_keeps_the_date_it_left_with, then trace Lexicon._stamps, _needs_stamp, _parse_time_records, _attach_stamp_baseline, and removed_entries(). Decide whether the stale-date behavior should change and review the proposed weak-reference design across the reader and writer. Done means the decision is implemented and the affected stamp tests reflect the chosen behavior.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- data
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100