Activity

1. imnasnainaec

   self-assigned this

   on Sep 25, 2026
2. imnasnainaec commented on Sep 25, 2026

   

   imnasnainaec

   on Sep 25, 2026

   Last edited by imnasnainaec

   CollaboratorAuthor

   More actions

   API surface review — python-sil-lift#30

   Evaluated 2026-09-25 on main @ 7b1dd32 plus #47 and #48, then rechecked 2026-10-08 on main @ 600de9e after both merged. The merged versions change no finding below; the only surface difference is that --allow also takes a comma-separated list.

   Question: are there functions, arguments, or user-doc details that no real user story needs? Every removal is free now and costs a major version after 1.0.

   Method

   • Surface: everything sil_lift.__all__ exports, with its public methods, attributes, and constructor arguments; the five CLI subcommands and their flags; the Action inputs.
   • User stories: the tasks the guides describe — read/edit/save, bulk edit, build an export, validate as a CI gate (Python, CLI, Action, container), stream large files, manage the folder (ranges, media, zip).
   • Evidence: where src/, tests/, and docs/ use each item. An item that only tests use, and that no guide or story reaches, is a candidate.

   Verdicts: Remove (recommended), Trim (keep it, shrink its contract), Decide (a judgment call, with a lean), Keep.

   Recommendations

   In priority order. R1–R3 are the ones worth acting on before 1.0.

   R1. Remove validate_file() and LiftValidationError — Remove

   • validate_file(path) is iter_problems(path) that raises on the first error. It is the only thing that raises LiftValidationError.
   • It has no require_ids argument, so it cannot back the one story where fail-fast matters most: a re-import gate. Adding one would be more surface for a two-liner.
   • The fail-fast story is covered already: the CLI's exit code for pipelines, and in Python if any(p.level == "error" for p in sil_lift.iter_problems(path)): ....
   • Usage: one line in docs/en/guides/validate.md, two tests.
   • Cost of removal: delete both names from __all__, the guide line, the two tests, and the CHANGELOG mention.

   R2. Make the model constructors keyword-only — Trim

   • Entry, Sense, Note, and most of the model are kw_only=True. Form, Trait, Annotation, URLRef, GrammaticalInfo, Span, Text, and Multitext are not, so their field order is public API: for example, Annotation(name, value, who, when, content, extra) positionally.
   • No story needs positional construction beyond the first field or two, and after 1.0 no field could be added or reordered without a major bump.
   • Lean: kw_only=True throughout, except for the leading required field where the positional form reads naturally (Form(lang, text), Trait(name, value), Text(fragments)). Docs and tests construct these positionally in a few places; those need updating.

   R3. Shrink Changes / RangesChanges to what a story reads — Decide (lean trim)

   • The one documented story is the write guard, if not lex.changes(): .... bulk-edit-glosses.md also uses changed_entries() for a count.
   • No guide or src/ consumer reads Changes.reordered, .header, .root, or .ranges, or anything on RangesChanges. Only tests do.
   • Yet those fields carry the densest contract in the package: one-way truthiness, baseline semantics, and how an entry aliased into the list twice is counted. The docstrings for Changes, RangesChanges, changed_entries(), added_entries(), removed_entries(), and changes() run to over 100 lines, and all of it becomes frozen.
   • Lean: keep Changes.__bool__, entries, added, removed, and baseline; make reordered, header, root, and ranges private (or keep them, documented as informational and exempt from SemVer). Keep RangesFile.changes(), which guards the standalone ranges-edit story, but return it as truthiness only.
   • Related: Lexicon.added_entries() and removed_entries() duplicate changes().added and .removed. Their only advantage is skipping serialization, and no story needs that. Lean: remove them, and point to changes().

   R4. Require ranges_file in Lexicon.add_ranges_file() — Trim

   • add_ranges_file(ranges_file=None, *, href=...) creates an empty RangesFile when none is passed. That companion gets no header references until the caller populates it and calls again, which the docstring itself has to explain.
   • The build-export guide always passes one. Only tests/test_unicode.py:331 uses the None form.
   • Lean: make ranges_file a required positional argument.

   R5. Streaming: two ways to open — Keep (document it)

   • open_reader / open_writer are pure pass-throughs to the LiftReader / LiftWriter constructors. Nothing outside the factories calls those constructors.
   • The classes must stay exported for type annotations, and the factories are what the docs teach, so remove neither.
   • Instead, say in the class docstrings to open with open_reader / open_writer, so the two spellings aren't read as distinct features.
   • LiftReader.producer is public but undocumented, and the large-files copy example passes a literal producer= rather than reader.producer. Mention it there or leave it; either is fine.

   Everything else — Keep

   Item	User story it serves
   load() and Lexicon.load()	Headline entry point, plus the classmethod idiom. A deliberate duplicate, like json.load next to a class loader
   load(resolve_ranges=False)	Skip folder discovery (stdin, canonicalize, speed)
   Lexicon.save(path, stamp=, when=)	Save, export a copy, reproducible CI output (and #9)
   Lexicon.save_zip(wrap_folder=str|bool)	FLEx and Combine packaging. The string form names the folder
   Lexicon.sort() and canonicalize()	Two different outcomes: a minimal-diff sort, or a fully canonical copy. Both documented
   Lexicon.find(id=, guid=)	Tour and guides
   Lexicon.changed_entries()	Bulk-edit guide
   Lexicon.iter_problems(require_ids=) and iter_problems(path, ...)	Validate before saving; validate a file
   Lexicon.all_ranges(), ranges_files, RangesFile.*	Folder guide
   Lexicon.media_refs(), check_media(), MediaRef, MediaResolution	Folder guide, and the check-media / validate media checks. MediaRef.sense_id has no src/ consumer, but it is how a caller finds the sense an illustration belongs to — cheap, keep
   RangesFile.add_range(), Range.add_element()	Build-export guide
   Entry.all_senses(), gloss_langs(), Sense.gloss()	Guides and the tour; the CLI's export uses gloss()
   Header.ranges_extra / fields_extra	Fidelity: residue on wrapper elements that have no model object. Must be carried
   extra= on every constructor	A dataclass artifact. Callers can't build meaningful Extras, but the fields must exist for fidelity. Not worth special-casing
   Extras.to_string()	Inspecting what sil-lift didn't understand. Tiny
   Problem and its fields	Validate guide, JSON output
   LiftError, LiftParseError, LiftWriteError	Version guard, lone-surrogate refusal, the CLI's exit 2
   __version__	Release process

   CLI and Action — Keep

   • validate and its flags each map to the CI-gate story: --format json, --strict, --no-check-media, --allow (Add validate --allow CODE to leave a code out of pass/fail #48), --require-ids, and - for stdin (an exporter piping straight in; tested).
   • check-media overlaps validate's missing-media and media-href-mismatch, but adds the orphan report, which nothing else provides.
   • stats --format json is the count-assertion story in lift-export-interop.md.
   • sort -o and export --langs --tsv -o: documented stories.
   • Action inputs: all used. There is a gap rather than excess: validate --require-ids has no Action input, though the interop guide pairs --require-ids with the re-import story. Out of scope here; worth its own issue if non-Python CI needs it.

   Docs details that serve no story

   • docs/en/guides/bulk-edit-glosses.md spends two bullets and seven sub-bullets on changed_entries() and changes(): baseline loss, aliasing, one-way truthiness, destination-vs-content, and cost. The script itself calls changed_entries() only for a count and never calls changes(). Lean: cut this to two bullets (what changed_entries() reports, and a pointer to changes() as the write guard). Move the guard material to read-edit-write.md, where saving is covered, trimmed to match R3.
   • Docstrings for the change-detection family (Changes, RangesChanges, changed_entries, added_entries, removed_entries, changes): the "entry aliased into the list twice" paragraphs describe a state no story creates. Keep the behavior pinned by tests, but one sentence in Changes would do. The changes() docstring's three-paragraph cost discussion could be one sentence ("costs more than the save it guards; it saves the write, not the work").
   • validate.md loses its validate_file example under R1. Nothing else there is surplus: the media-mismatch rules, the companion-scope rules, and the FLEx policy each answer a question a CI user hits.
   • cli.md opens by calling the CLI "a worked example of the library API" for validate. That is true of the code, but tells a CLI reader nothing. Optional cut.

   Not surplus, despite appearances

   • The media and companion folding helpers are all private (_model), so Report a media href that reaches its file only under case folding #47's additions add no public surface beyond one problem code, which the CI story needs.
   • Add validate --allow CODE to leave a code out of pass/fail #48 adds one flag (repeatable, and taking a comma-separated list), one JSON summary key, and one Action input, all on the CI-gate story.

   ---

   Authored by Claude Opus 5.5 (claude-opus-5-5) via Claude Code.