Deprecate binary en/he TextChunk for real-language TextChunk

[YishaiGlasner]

Naming note: the new real-language class was originally called TextRange and renamed to TextChunk partway through this work, with the old binary class becoming LegacyTextChunk. This description uses the final names throughout — TextChunk always means the new, real-language class; LegacyTextChunk always means the old binary en/he one.

LegacyTextChunk was built on a binary en/he assumption: a version's real language (Yiddish, French, Judeo-Arabic, etc.) got forced into an "en"/"he" bucket by direction, so a German translation stored as language="en" would be returned to callers as if it were literally English. This PR does two things.

1. Gives the new TextChunk save support and wires it end-to-end, including the editor

TextChunk.save() mirrors LegacyTextChunk.save()'s architecture directly — it inherits AbstractTextRecord and reuses _validate, _sanitize, _trim_ending_whitespace, _check_available_text_pre_save, _update_link_language_availability unchanged. sefaria/tracker.py's modify_text — the shared entry point for every text save — now constructs the new TextChunk (actual_lang/direction threaded through from its own new direction kwarg) instead of the old one; this is where the save path actually moved. What's different, and why:

• New constructor params, direction and actual_lang, alongside lang/vtitle. lang (languageFamilyName) can now be derived from actual_lang via the existing LANGUAGE_CODES map, so a caller creating a brand-new version only needs to supply the real ISO code an editor picked — not compute both independently.
• direction is a new lookup/save dimension, and also the source for the legacy language field ("he" if direction == "rtl" else "en") when a brand-new Version is created, since that field still has to satisfy the old binary schema underneath.
• What "saveable" means changed. LegacyTextChunk was saveable whenever lang and vtitle were both given, gated further by fallback_on_default_version/exclude_copyrighted to guard its implicit/fuzzy vtitle-guessing mode. The new TextChunk never has that fuzzy mode — it's saveable whenever vtitle is given plus either direction or lang (not both required), with no fallback/guessing params. (exclude_copyrighted was also confirmed dead on LegacyTextChunk itself — never passed True anywhere — and dropped from Ref.text()'s public signature entirely; grepped both this branch and master for any caller passing it, none found.)
• Version.pkeys changed from ["title", "versionTitle"] to ["title", "direction", "versionTitle"]. This isn't a hypothetical concern: the live data already has real collisions on the old key. We found 60 (title, versionTitle) pairs that currently exist as both an ltr and an rtl version — e.g. "Rashi on Psalms" / "Wikisource Mikraot Gedolot" exists in both directions. Whether that data pattern itself is desirable is a separate question this PR doesn't touch — we didn't create or change it, we only changed pkeys to correctly reflect the uniqueness boundary the data already has, instead of one that was already silently violated.
• TextChunk._validate() is narrower than LegacyTextChunk._validate() — it only checks that the posted text's depth matches the ref, dropping the old spanning-ref/range-length validation branches entirely. This is safe, not an oversight: _saveable excludes is_range() refs outright, so a spanning or range ref can never reach save() on the new class in the first place — there's nothing left for those branches to validate.
• The [xx]-versionTitle-suffix convention is kept, not retired — db.history and some legacy version-grouping code still key on the coarse (language, versionTitle) pair, so a German version forced into the language="en" bucket needs the suffix to avoid colliding with an actually-English version sharing that title. It's now generated automatically on save instead of something an editor had to type manually. Because the save response doesn't otherwise expose the possibly-auto-suffixed title, TextChunk.save() sets self.vtitle from the saved Version's real title before returning, and the save API echoes it back in its JSON response — without this, a caller holding the pre-suffix title (e.g. a second save in the same session, or a script doing sequential per-segment saves) could create a colliding duplicate version.
• Editor (editor.js/edit_text.html): the hardcoded <select> with two options (en/he) is replaced with an ISO-code picker driven by Sefaria.ISOMap (same source used elsewhere in the client). There's no separate direction control: direction is derived from Sefaria.ISOMap[lang].defaultDirection for a brand-new version, and left completely alone when editing an existing one, so a version whose direction was deliberately saved as an exception to its language's default (a handful of real transliterated Arabic/Persian versions) is never silently overwritten. Adding/editing a "Sefaria Community Translation" hides the language picker and forces English, since that identity is fixed by definition. The /edit/<ref>/<lang>/<version> URL and its client-side builder (sefaria/urls_library.py's route regex, ConnectionsPanel.jsx's edit-link construction) both moved from a 2-letter \w\w (en/he only) segment to a full languageFamilyName string, to carry a real language through the URL instead of a binary code.

2. Renames the classes, moves the legacy ones out, and migrates every live caller to the new TextChunk

This starts with the rename itself — TextRange → TextChunk, old TextChunk → LegacyTextChunk — and physically relocating LegacyTextChunk, VirtualTextChunk (its delegate for virtual/dictionary nodes), and TextFamily into a new sefaria/model/legacy_text.py, no longer exported from sefaria/model/__init__.py.

TextFamily is worth calling out specifically — it was the bridge/assembly class that built the full legacy response shape (text plus commentary, links, and version metadata bundled together) for the old v1 texts API and everything that rendered off of it. It was a heavily-used object across the old GET path, not a minor helper. It's kept around for the legacy v1 texts_api GET (external third-party consumers) — see the class docstring on LegacyTextChunk for the full breakdown of what else still technically routes to it and why. reader/views.py's social_image_api is fully migrated off TextFamily entirely (ref.padded_ref().text(direction=...)) — it no longer needs it at all. (sefaria/image_generator.py is unrelated to this migration — its only change in this PR is folding an existing ternary into the shared direction/lang helper described below; it never used TextChunk/TextFamily.)

With the rename and relocation in place, every live caller of the old class is migrated. Three categories:

Migrated to direction= — deliberately preserving the old bucket-based behavior, for call sites whose output is implicitly a closed "he"/"en" pair or that are legacy-keyed and shouldn't change behavior: client/wrapper.py's get_links(), linker.py's _get_ref_text_by_lang_for_linker, marked_up_text_chunk.py, marked_up_text_chunk_generator.py, disambiguator.py, sourcesheets/views.py's get_correct_text_from_source_obj, reader/views.py's social_image_api, history.py's text_at_revision (matches db.history's own legacy language-field keying), search.py's index_ref, user_profile.py's recently-read text preview.

Migrated to real actualLanguage matching — deliberately closing the leak, where the caller's intent was genuinely "real English" or "real Hebrew," not "whatever's in the direction bucket": Ref.word_count() (no live callers found anywhere, migrated anyway rather than leaving a redundant legacy-only path around). api/views.py's KnnSearch._ref_text also belongs here, with a wrinkle worth flagging explicitly: its diff against master is empty. The call site reads Ref(ref).text(lang="he"), textually identical to the line on master — but lang= means something different now (exact actualLanguage match, not the old positional binary bucket), so its behavior differs from master even though nothing shows up in a file diff.

Moved away from language-bucketed logic entirely — behavior unchanged: _build_links_internal (sefaria/helper/link.py) no longer touches TextChunk/TextFamily at all — its TextFamily(...).contents()-based non-emptiness check is replaced with Ref.is_empty(vstate=vstate)/get_state_ja(vstate=vstate)/get_subrefs_count(...), none of which take a language argument. This is equivalent to the old behavior, but not because the new check is language-agnostic — it isn't. VersionState's _all mask (sefaria/model/version_state.py:219) is literally _en + _he, built from the same legacy Version.language field TextFamily read from. Both the old and new checks are equally blind to real non-en/he-languaged versions; this migration just stops routing that (pre-existing, unchanged) blind spot through legacy field names. Two structural helpers in sefaria/helper/schema.py (used when converting between simple/complex text structures) similarly construct the new TextChunk directly from an already-loaded Version's own lang/direction/vtitle — no bucket decision needed there at all, since the version is already known.

Completely removed: the alt-structure text preview (SchemaNode.as_index_contents()'s wholeRefPreview/refsPreview generation, expand_ref) — confirmed dead, its only consumer was the pre-React templates/js/headers.js, itself gated behind an already-unused flag.

Marked as legacy, not removed (beyond TextFamily above): it's for the v1 API — see the LegacyTextChunk class docstring (sefaria/model/legacy_text.py) for what else still technically routes to it and why none of that is a real reason to keep the class around. The legacy v1 texts_api POST now derives a sensible direction from language when a caller doesn't send it, so older API clients keep working without needing to be updated themselves. LegacyTextChunk's own internal availability-check helpers construct LegacyTextChunk directly rather than going through the new Ref.text(), so they keep matching the whole legacy bucket rather than one exact language.

Won't work, for a different reason than it might look like: four scripts (find_replace.py, fix_rashi_double_quotes.py, remove_marks_from_tyt.py, replace_stuma_and_ptuha.py) were updated — not because they construct TextChunk directly, but because sefaria/helper/text.py's find_and_replace_in_text()/modify_text_by_function() had their positional signature changed (lang → direction) as real Phase 1/2 migration work, and these scripts call those functions directly. Without updating them, they'd silently pass "he" where "rtl" is now expected. Separately, roughly 20 other one-off analysis scripts under scripts/ still construct TextChunk(oref, "he"/"en", vtitle) directly, using the pre-rename 3-positional-argument convention — since TextChunk now refers to the new class, the literal "he"/"en" is read as languageFamilyName and matches nothing. These were left as-is: none are wired into any scheduled job, only invoked by hand, so updating them was judged not worth the review burden until someone actually needs to run one.

Deviation from the original plan worth flagging: sefaria/helper/text.py's split_text_section was planned for deletion (confirmed zero external callers — only its own recursive self-call). It was migrated instead of deleted. Still zero callers as of this PR — a candidate for actual removal as a follow-up.

Bugs fixed (pre-existing, not introduced by this PR)

• merge_texts() had two bugs, both already reachable in production through LegacyTextChunk._choose_version_by_lang (used throughout the app for any multi-version merge): depth>2 merges flattened the source-attribution list across the entire node, losing which position each source belonged to (the code's own comment already flagged this: "the mapping of source names to segments is lost for merged texts of depth > 2"); and empty positions defaulted to attributing the first candidate version instead of being left unattributed, fabricating version credit for content that version never actually had. Fixed by preserving nesting through the merge and using an explicit "no attribution" sentinel for empty positions. export.py's merge visitor (used for exporting a merged text) was updated to match the corrected, now-nested shape.
• The v3 texts API reported a version as "merged from multiple versions" (TextRequestAdapter._append_version, read by the client as currentVersion.merged = !!(currentVersion.sources)) whenever 2+ candidate versions existed for a language, even when the requested version alone was already complete for that ref — surfacing a misleading "merged" badge on an ordinary single-version read. Fixed to only report it on genuine multi-version attribution.

Performance

• TextRequestAdapter._add_ref_data_to_return_obj now shares a single VersionState fetch to compute next/prev section links, instead of two independent uncached DB round-trips. This isn't editor-specific — _add_ref_data_to_return_obj is shared by edit_text, the /api/v3/texts endpoint, and the main reader's own SSR panel loading (make_panel_dict), so this benefits ordinary reader page loads too, not just the editor.

Testing

• New regression tests, all synthetic fixtures (no dependency on real book data that could change): merge_texts()'s positional-correspondence/no-fake-attribution fix (chunk_test.py), the version-title-suffix save fix (chunk_test.py), the v3 API's false-merge-flag fix — both the no-false-positive and still-flags-genuine-merges cases (new text_request_adapter_test.py), get_links()'s version-metadata attribution (links_test.py), and edit_text's response shape — isPrimary/isEdited flags and position metadata (reader/tests.py).
• TextChunk read/save coverage parity with LegacyTextChunk (chunk_test.py, 6 new tests, LegacyTextChunk's own tests untouched): LegacyTextChunk had comprehensive read-path coverage (verse/chapter/range/spanning refs, depth-1, nested commentary-style addressing) and an extensive save-path test (blank writes, extending beyond current extent, writing within extent, HTML sanitization, whole-chapter overwrite with blank trimming, depth-3 saves) — but that save-path test only ever exercised LegacyTextChunk.save(), which is no longer the live save path. Added equivalent coverage for the new TextChunk, all synthetic fixtures.
• Also updated three test files that built fixtures via LegacyTextChunk even though the production code they exercise was already migrated to TextChunk (sefaria/tests/search.py's test_make_text_index_document, search_dependencies_test.py's book fixture, a linker_test.py validation helper) — consistency, not a behavior change.
• resolve_default_version() guarded against an explicit priority=None (text_request_adapter.py) — Version._normalize() can set priority to None on a float() parse failure, not just leave it missing; getattr(v, 'priority', 0)'s default only covers the latter, so max() could raise TypeError comparing None to a number. Pre-existing on master (carried over unchanged when this logic was extracted into the new helper), fixed here since we were already in this function.
• Full existing suite run clean: chunk_test.py, text_test.py, schema_test.py, auto_linking_test.py, linker_test.py, links_test.py, social_image_api_test.py, search_dependencies_test.py (20 tests against the real indexing pipeline via a fake ES client) — no regressions.
• Manually checked already:
  • /api/texts and /api/v3/texts for a real ref, /api/links?with_text=1 on a heavily-commented ref, /api/img-gen for en/he (viewed the rendered image).
  • A full delete+regenerate of every auto-generated Genesis/Rashi-on-Genesis link (2018 links, confirmed byte-identical before vs. after).
  • "Gur Aryeh on Shemot" (depth≥3, two English versions) — merged reading in the reader and merged-text export/download both work correctly.
  • Adding a source to a sheet with a specific version selected (sourcesheets/views.py's get_correct_text_from_source_obj).

Merging with master

This branch was rebased against a moving master partway through review. One real semantic conflict, in Ref.is_text_fully_available(): master had independently added a third branch (handling refs to whole branching/structural nodes, e.g. a Sifra parsha, via _aggregate_structure_state) that didn't exist when this branch diverged. Resolved by keeping this PR's direction=-based migration for the segment/section-level branch (also dropping a try/except NoVersionFoundError that the new TextChunk doesn't need — confirmed no code path reaches it, since the new class returns empty text gracefully instead of raising) while preserving master's new structural-node branch intact. Verified all three branches work post-merge against real refs (a plain segment/section ref, a simple book-level JaggedArrayNode ref, and a complex structural ref like "Pesach Haggadah").

Two master-authored tests needed updates for compatibility with this PR's real-language semantics — not regressions in their own original intent, confirmed by checking both against origin/prod directly (prod predates this PR's changes, and both pass there):

• modtools_test.py::test_rename_collision_different_language_family_allowed — asserted a literal versionTitle string for a non-en/he version that this PR's auto-suffix mechanism now rewrites on save (by design, to prevent exactly the collision the test was checking couldn't happen). Updated to assert the real, suffixed behavior.
• text_test.py::test_index_rename_migrates_versions — its fixture inserts Version docs via raw db.texts.insert_many(), bypassing Version._normalize() and therefore actualLanguage/languageFamilyName, which Ref.text(lang=...) now matches on instead of the legacy binary language field. Added both fields to the fixture.

Three other CI failures reported during review are unrelated pre-existing breakage, confirmed independently of this branch (checked against a clean master/prod checkout) and intentionally left untouched: linker_editor_test.py::test_usage_index_surgical_add_remove (new, unreleased feature, never had a clean run), and search_dependencies_test.py's test_hooks_noop_when_search_index_on_save_off/test_category_hook_empty_new_path_guard (a naming mismatch between a test merged 2026-08-19 and an unrelated dependencies.py refactor merged one week later, already broken on prod).

What should be manually checked before merge

• Saving/editing through the UI (existing English/Hebrew versions, a new non-en/he version, the "Add Translation" and "Original Translation" flows) — we've verified this via unit tests and targeted manual checks, but hope the content team can also run through real editing sessions before this ships, since that's the surface with the most user-facing change.
• The linker widget (linker.v2.js-embedded pages) — spot-check that it still returns text for a few refs, including one needing merge/gap-fill across versions.
• Topics/LLM pipeline — topic_prompt.py consumes get_links(with_text=True) and calls .text() directly; worth confirming topic-prompt generation still produces correct source text.

🤖 Generated with Claude Code

[gitvelocity-reviewer]

I'll analyze this PR systematically, reviewing the code changes, architecture, implementation quality, and test coverage.

PR Overview

This PR migrates Sefaria's text access layer from a binary en/he language model to a direction-based (ltr/rtl) model. It extracts TextChunk, VirtualTextChunk, and TextFamily into a new legacy_text.py module, updates call sites across the codebase, and refactors the editor UI to support real language codes.

---

Issues Found

1. Potential Bug: tracker.modify_text signature change breaks callers

File: sefaria/tracker.py

def modify_text(user, oref, vtitle, lang, text, vsource=None, direction=None, **kwargs):
    chunk = model.TextChunk(oref, actual_lang=lang, vtitle=vtitle, direction=direction)

The lang parameter is now treated as an ISO language code (actual_lang), but many callers still pass "en" or "he" as the legacy bucket. For example, in sefaria/helper/text.py:

modify_text(user, ref, vtitle, lang, text, vsource, direction=direction)

Where lang is still self.version_info['info']['language'] — a legacy "en"/"he" value. Passing "en" as actual_lang to TextChunk may work coincidentally, but it's semantically wrong and could cause subtle version-matching failures for non-English LTR languages.

2. Missing direction in tracker.modify_text calls from helper/link.py

File: sefaria/helper/link.py

The refactored _build_links_internal no longer passes text content, but the tracker.modify_text calls elsewhere that go through this path don't always supply direction. This could cause direction=None to be passed to TextChunk, which may fail or silently use a default.

3. split_text_section sets actual_lang from old_chunk.version().actualLanguage

File: sefaria/helper/text.py

new_chunk.actual_lang = new_chunk.actual_lang or old_chunk.version().actualLanguage

TextChunk (the new one) may not have an actual_lang attribute at all — this would silently set an attribute on the object without it being used by the underlying save logic, depending on how TextChunk.save() works.

4. sefaria/views.py — bundle_many_texts uses oref.text() with positional arg

File: sefaria/views.py

en_tc = oref.text(translation_language_preference, vtitle=english_version) if translation_language_preference \
    else oref.text(direction="ltr", vtitle=english_version)

oref.text() is called with translation_language_preference as a positional argument. The signature of Ref.text() needs to accept this as the first positional arg (likely direction or lang). If translation_language_preference is an ISO code like "fr", this would be passed as direction, which is wrong.

5. edit_text view — edited['direction'] KeyError risk

File: reader/views.py

text["edit_lang"] = get_legacy_lang_from_direction(edited['direction']) if edited else request.contentLang

If edited is not None but the version dict doesn't have a 'direction' key (e.g., older version records), this will raise a KeyError. Should use .get('direction', 'ltr').

6. get_links — NoVersionFoundError raised inside a loop that catches broadly

File: sefaria/client/wrapper.py

if not top_nref_tc._version_candidates and VersionSet({"title": top_oref.index.title}).count() == 0:
    raise NoVersionFoundError("No text record found for '{}'".format(top_oref.index.title))

This raise is inside a loop that's inside get_links(). The outer exception handling in get_links may swallow this, silently returning incomplete link data rather than surfacing the error.

7. sefaria/export.py — flatten_jagged_array import added but may not exist

File: sefaria/export.py

from sefaria.utils.util import flatten_jagged_array

This function needs to exist in sefaria/utils/util.py. The diff doesn't show this file being modified, so it's unclear if flatten_jagged_array was already there or needs to be added.

8. editor.js — #direction select not hidden for "original" mode

File: static/css/reader.css

#addVersionHeader.original #languageForm,
#addVersionHeader.original #directionForm {
    display: none;
}

This is correctly added. ✓

9. editor.js — readNewVersion() sends both language and direction

File: static/js/s1/editor.js

version["language"] = $("#language").val();
version["direction"] = $("#direction").val();

The server-side texts_api now derives direction from t.get("direction") or get_direction_from_legacy_lang(t["language"]). This is correct for backward compatibility. ✓

10. Test: EditTextViewTest._get_init_json is fragile

File: reader/tests.py

idx = html.index("current: ") + len("current: ")
data, _ = json.JSONDecoder().raw_decode(html, idx)

This parses the HTML response by string-searching for "current: ". If the template changes the variable name or whitespace, this test silently breaks. A more robust approach would be to use a dedicated test endpoint or parse the JSON from a known marker.

---

Test Coverage Assessment

The PR adds substantial tests:

• EditTextViewTest (2 integration tests for edit_text view)
• test_sources_scoped_and_no_fake_attribution_for_depth_gt_2 (chunk sources correctness)
• test_save_reuses_existing_version_despite_stale_unsuffixed_vtitle (version suffix behavior)
• test_append_version_no_false_merge_flag_when_requested_version_is_complete (adapter merge flag)
• test_append_version_reports_sources_on_genuine_multi_version_merge (adapter merge flag)
• test_get_links_version_metadata_is_real_even_without_content_at_position (links regression)

Coverage is reasonable for the new behavior. The legacy_text.py module itself is largely moved code (existing tests cover it via LegacyTextChunk renames). The tracker.modify_text signature change and bundle_many_texts oref.text() positional arg issue are not covered by tests.

Estimated coverage: ~65% — slightly below the 70% threshold for the new logic paths, particularly around tracker.modify_text with the new direction parameter and the split_text_section actual_lang assignment.

---

Code Quality Observations

Positives:

• The legacy_text.py extraction is well-motivated and clearly documented
• get_direction_from_legacy_lang / get_legacy_lang_from_direction helpers are clean and centralized
• The TextRequestAdapter fix for false merge flags is well-tested
• Comments throughout explain the legacy/new boundary clearly
• The _add_ref_data_to_return_obj optimization (sharing one VersionState fetch) is a good catch

Concerns:

• The tracker.modify_text lang vs actual_lang semantic confusion is the most dangerous issue
• The oref.text(translation_language_preference, ...) positional arg call in views.py needs verification
• edited['direction'] KeyError risk in edit_text view

---

Code Quality Score

Sub-scores:

• S (Scope): 18/20 — 51 files across model, views, helpers, tests, JS, CSS, scripts, URLs. New legacy_text.py module, new text_request_adapter_test.py. System-wide migration touching every text-access call site.

• A (Architecture): 17/20 — Extracts LegacyTextChunk, VirtualTextChunk, TextFamily into legacy_text.py. Introduces direction-based API as the new primary interface. Adds resolve_default_version helper. Removes TextRange usage in favor of TextChunk. Changes __init__.py exports. Significant boundary change between legacy and new text access.

• I (Implementation): 15/20 — Migration of binary en/he to direction across 40+ call sites. _build_links_internal refactor removes text fetching. TextRequestAdapter merge-flag fix. _add_ref_data_to_return_obj VersionState sharing. edit_text view rewrite with isPrimary/isEdited flags. editor.js language/direction decoupling.

• R (Risk): 14/20 — Touches the core text-access path used by every reader, API, search indexer, and export. tracker.modify_text signature change affects all text saves. No feature flag. The lang vs actual_lang confusion in tracker.py is a live risk. External API consumers of texts_api now get versionTitle in the response (additive, low risk).

• Q (Quality): 10/15 — Good new tests for the specific bugs fixed. LegacyTextChunk rename tests are mechanical. Missing coverage for tracker.modify_text direction path, bundle_many_texts positional arg, split_text_section actual_lang. EditTextViewTest._get_init_json is fragile.

• P (Performance/Security): 3/5 — VersionState sharing in _add_ref_data_to_return_obj saves 2 DB queries per request. resolve_default_version is in-memory. No security concerns introduced.

Base Score: 18 + 17 + 15 + 14 + 10 + 3 = 77

Effort Scale:

• Effective Lines: 2934 → Extra Large tier (ESF: 1.0x)
• File Count: 51 → Extra Large tier
• No bump needed (already at max)

Final Score: 77 × 1.0 = 77

Code Quality Data (JSON)

{
  "_schema": "code_quality_v7",
  "summary": "This PR migrates Sefaria's text access layer from a binary en/he language model to a direction-based (ltr/rtl) model, extracting LegacyTextChunk, VirtualTextChunk, and TextFamily into a new legacy_text.py module. It updates 40+ call sites across model, views, helpers, scripts, and JS, and refactors the editor UI to support real ISO language codes alongside the direction toggle.",
  "total_score": 77,
  "total_factors": "77 × 1.0 (Extra Large ESF) = 77",
  "scope_score": 18,
  "scope_factors": "51 files touched across sefaria/model/ (text.py, legacy_text.py new, text_request_adapter.py, schema.py, garden.py, marked_up_text_chunk.py, user_profile.py), reader/views.py, sefaria/views.py, sourcesheets/views.py, sefaria/tracker.py, sefaria/search.py, sefaria/export.py, sefaria/history.py, sefaria/sheets.py, sefaria/client/wrapper.py, sefaria/helper/text.py, sefaria/helper/link.py, sefaria/helper/schema.py, sefaria/helper/linker/, sefaria/helper/marked_up_text_chunk_generator.py, sefaria/constants/model.py, static/js/s1/editor.js, static/css/reader.css, templates/edit_text.html, sefaria/urls_library.py, sefaria/urls_shared.py, and multiple scripts and test files. New files: sefaria/model/legacy_text.py (865 lines), sefaria/model/tests/text_request_adapter_test.py.",
  "architecture_score": 17,
  "architecture_factors": "LegacyTextChunk, VirtualTextChunk, and TextFamily are extracted from text.py into the new legacy_text.py module, with sefaria/model/__init__.py updated to remove TextFamily and TextRange from its public exports. The new direction-based TextChunk becomes the primary interface; legacy callers are redirected through get_direction_from_legacy_lang() and get_legacy_lang_from_direction() helpers added to sefaria/constants/model.py. resolve_default_version() is introduced in text_request_adapter.py as an in-memory version resolver. TextRange usage in _append_version is replaced by TextChunk. The edit_text URL pattern changes its capture group from lang (2-char) to language_family_name (word chars), and the view signature changes accordingly.",
  "architecture_score": 17,
  "implementation_score": 15,
  "implementation_factors": "_build_links_internal in sefaria/helper/link.py is refactored to remove TextFamily fetches, instead using oref.get_state_ja(vstate=vstate) and oref.is_empty(vstate=vstate) to determine link targets without loading text content. edit_text in reader/views.py is rewritten to use TextRequestAdapter with isPrimary/isEdited flags on version dicts, and to restore sections/toSections from the original unzoomed ref after fetching at context_ref level. _add_ref_data_to_return_obj in text_request_adapter.py shares one StateNode/VersionState fetch between next_section_ref and prev_section_ref calls. _append_version filters sources to suppress false-merge flags when a single candidate covers all positions. editor.js decouples the language select (ISO code) from the direction select (ltr/rtl), populates #language from Sefaria.ISOMap at init, and rewrites sjs.editText to use versions[].isEdited and versions[].isPrimary rather than the legacy he/text split.",
  "risk_score": 14,
  "risk_factors": "tracker.modify_text's signature changes: lang is now passed as actual_lang to TextChunk, but many callers still supply the legacy 'en'/'he' bucket string rather than a real ISO code, creating a semantic mismatch that could cause version-matching failures for non-English LTR languages. The edit_text view accesses edited['direction'] without .get(), risking KeyError on older version records that lack the field. bundle_many_texts in sefaria/views.py calls oref.text(translation_language_preference, ...) with translation_language_preference as a positional argument, which may be interpreted as direction rather than an ISO lang code. The texts_api POST path now returns versionTitle in the response body (additive). No feature flag gates the migration; all text-save and text-read paths are affected immediately.",
  "quality_score": 10,
  "quality_factors": "reader/tests.py adds EditTextViewTest with two integration tests covering isPrimary/isEdited flags and sections/toSections position metadata. sefaria/model/tests/chunk_test.py adds test_sources_scoped_and_no_fake_attribution_for_depth_gt_2 and test_save_reuses_existing_version_despite_st

[Copilot]

Pull request overview

This PR migrates Sefaria’s text read/write path away from the legacy binary "en"/"he" TextChunk model to a real-language-aware TextChunk (using actualLanguage + languageFamilyName + direction), and updates the editor and major call sites accordingly while retaining legacy API surfaces via a new legacy_text module.

Changes:

• Replaces the old binary TextChunk with a new real-language TextChunk (read + save), updates Ref.text()/tracker.modify_text() to use it, and adjusts Version uniqueness to include direction.
• Moves legacy read-path classes (LegacyTextChunk, TextFamily, etc.) into sefaria/model/legacy_text.py and migrates most internal callers to direction= / actualLanguage semantics.
• Updates the editing UI to select ISO language + direction, and adds/updates regression tests around merging/sources attribution and editor response shape.

Reviewed changes

Copilot reviewed 51 out of 51 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
templates/edit_text.html	Replaces hardcoded en/he language picker with ISO-language select and adds a direction picker.
static/js/s1/editor.js	Populates ISO language list in the editor; uses versions metadata (isPrimary/isEdited, actualLanguage, direction) for edit/compare flows.
static/js/ConnectionsPanel.jsx	Updates edit URL construction to use languageFamilyName instead of en/he.
static/css/reader.css	Hides the new direction selector for “Original Translation” mode.
sourcesheets/views.py	Switches sheet source text retrieval to Ref.text(direction=...) while preserving en/he sheet semantics.
sefaria/views.py	Updates bundled-text code paths to use direction/actual-language selection instead of legacy chunks.
sefaria/urls_shared.py	Adds clarifying “legacy” comments for older endpoints.
sefaria/urls_library.py	Updates edit URL route to accept language_family_name instead of 2-letter lang.
sefaria/tracker.py	Routes saves through new TextChunk(actual_lang=..., direction=...) and threads direction through modify path.
sefaria/tests/search.py	Updates search test to use LegacyTextChunk where legacy behavior is required.
sefaria/tests/search_dependencies_test.py	Updates test fixtures to use LegacyTextChunk and clarifies VersionState refresh behavior.
sefaria/tests/links_test.py	Adds regression test around link version metadata attribution when text is empty at a position.
sefaria/sheets.py	Keeps dormant legacy flows on LegacyTextChunk with explicit legacy imports.
sefaria/search.py	Indexing pulls text via new TextChunk keyed by direction derived from legacy lang.
sefaria/model/user_profile.py	Recently-read preview now uses ref.text(direction=...) instead of legacy chunk reads.
sefaria/model/text.py	Core migration: new TextChunk, Ref.text() signature changes, Version.pkeys includes direction, fixes to merge_texts() attribution nesting.
sefaria/model/text_request_adapter.py	Uses new TextChunk in v3 adapter, refines merge/sources reporting, and reduces VersionState DB reads.
sefaria/model/tests/text_test.py	Updates tests to use LegacyTextChunk where appropriate after rename/migration.
sefaria/model/tests/text_request_adapter_test.py	New tests covering v3 adapter “sources” behavior (no false merges; still flags real merges).
sefaria/model/tests/schema_test.py	Updates schema tests to use LegacyTextChunk in legacy expectations.
sefaria/model/tests/index_schema_test.py	Updates index schema tests to use LegacyTextChunk.
sefaria/model/tests/index_offsets_by_depth_tests.py	Imports legacy TextFamily for tests that still rely on it.
sefaria/model/tests/chunk_test.py	Adds regression tests for merge attribution correctness and vtitle suffixing on save; updates legacy chunk usage.
sefaria/model/schema.py	Removes dead alt-structure preview generation previously routed through legacy text objects.
sefaria/model/marked_up_text_chunk.py	Validates MUTCs using oref.text(direction=...) instead of legacy chunk construction.
sefaria/model/linker/tests/linker_test.py	Uses LegacyTextChunk in linker validation test fixtures.
sefaria/model/legacy_text.py	New module containing LegacyTextChunk, VirtualTextChunk, and TextFamily moved out of text.py.
sefaria/model/garden.py	Keeps garden feature on LegacyTextChunk explicitly.
sefaria/model/init.py	Stops exporting legacy classes from sefaria.model root import surface.
sefaria/image_generator.py	Refactors legacy lang→direction conversion via shared helper.
sefaria/history.py	Reads revision text via direction-derived TextChunk for legacy history language keying.
sefaria/helper/text.py	Changes helper APIs from lang to direction for write operations and threads direction into modify/save flows.
sefaria/helper/tests/text_test.py	Updates helper tests to match the new helper API signatures and legacy chunk usage.
sefaria/helper/tests/schema_test.py	Updates schema helper tests to use LegacyTextChunk.
sefaria/helper/tests/auto_linking_test.py	Updates auto-linking tests to use LegacyTextChunk where the legacy path is required.
sefaria/helper/schema.py	Constructs new TextChunk directly from Version metadata (family + direction) during schema migrations.
sefaria/helper/marked_up_text_chunk_generator.py	Loads segment text via direction derived from legacy lang.
sefaria/helper/linker/linker.py	Linker text fetch now uses oref.text(direction=...) merge behavior rather than legacy fallback mode.
sefaria/helper/linker/disambiguator.py	Disambiguator uses direction-based fallback behavior when scoring candidate refs.
sefaria/helper/link.py	Removes dependency on TextFamily for link-building logic, using VersionState/ref emptiness checks instead.
sefaria/export.py	Updates merged export metadata collection to flatten nested merged source structures.
sefaria/datatype/jagged_array.py	Updates comments to reference LegacyTextChunk.trim_text as the source.
sefaria/constants/model.py	Adds shared helpers for legacy lang ↔ direction conversion.
sefaria/client/wrapper.py	get_links(with_text=1) migrated off TextFamily and improves version attribution handling for empty positions.
scripts/replace_stuma_and_ptuha.py	Updates helper call sites to pass direction instead of legacy lang.
scripts/remove_marks_from_tyt.py	Updates helper call sites to pass direction instead of legacy lang.
scripts/fix_rashi_double_quotes.py	Updates helper call sites to pass direction instead of legacy lang.
scripts/find_replace.py	Updates helper call sites to pass direction instead of legacy lang.
reader/views.py	Editor view now uses v3 adapter response shape and threads direction into legacy v1 POST saves; social image API migrated off TextFamily.
reader/tests/social_image_api_test.py	Updates social image tests to mock the new ref.text(...) path rather than TextFamily.
reader/tests.py	Adds regression tests for edit_text() response flags/metadata; updates remaining uses to legacy chunk where required.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[nsantacruz]

You can use the new TextChunk here. No reason not to.

[nsantacruz]

why use LegacyTextChunk here?

[stevekaplan123]

Can you add to the PR a description of why we need LegacyTextChunk? I see that it's used in gardens, but I find its existence confusing.

[YishaiGlasner]

Can you add to the PR a description of why we need LegacyTextChunk? I see that it's used in gardens, but I find its existence confusing.

@stevekaplan123 added comment to LegacyTextChnk and edoted desc