feat(sc-46669): let siddur instruction rubrics span both columns

[EliezerIsrael]

Closes sc-46669

Siddur texts carry English rubrics in <i class="instruction"> tags — "Some say the following meditation before putting on the tefillin." They live on the translation side, in segments that have no source text at all.

Example: https://www.sefaria.org/The_Koren_Shalem_Siddur%3B_Ashkenaz%2C_Weekdays%2C_Tefillin.1?lang=bi

Before / after

mode	before	after
Bilingual side-by-side	rubric wraps inside the 50% translation column, empty column beside it	rubric spans both columns
Bilingual stacked	—	unchanged
Translation-only	—	unchanged
Source-only + English UI	segment renders empty (height 0)	rubric shown
Source-only + Hebrew UI	hidden	unchanged (hidden)

Why the trigger is "no source text", not "has an instruction tag"

Keying off :has(> i.instruction) alone misfires. Sampling 11 siddur sections via /api/v3/texts (498 segments, 174 containing instructions):

pattern	count
EN instruction-only, tag at top level	98
EN instruction mixed with real translation, tag at top level	32
HE instruction (always mixed, never alone)	58

Those 32 include Tefillin 11 ("Some say:" + a full translation) and every Leader: / Cong. then Leader: line in Kaddish and Kedusha — all with facing Hebrew. Full-widthing them would break the parallel layout.

Of 102 segments with no Hebrew, 101 carry a rubric, and zero instruction-only translations have facing Hebrew. So this exposes the already-computed hasNoPrimarry as a noPrimary class and keys the CSS on that. The source-only rule additionally requires :has(> i.instruction) so missing-source segments library-wide don't start showing English in Hebrew-only mode.

SID-002 is the regression guard for the mixed case — without it a :has(i.instruction) implementation would look correct.

Notes for review

• Both rules are scoped to .basetext, so the connections panel is unaffected (TextList/VersionsTextList pass basetext={false}).
• Selectors key on .primary/.translation rather than .he/.en, because those language classes are currently unreliable on this text — see sc-46469, fixed in the follow-up PR stacked on this branch.
• :has() already appears 8× in these stylesheets.

Tests

e2e-tests/library/siddur-instruction-rubrics.spec.ts — SID-001 … SID-004, plus new SourceTextPage methods (setBiLayout, getSegmentSpanLayout, isInstructionRendered).

🤖 Generated with Claude Code

[gitvelocity-reviewer]

📊 Code Quality Score: 23/100

Base 38 × ESF 0.6 = 22.8, rounded to 23

Category	Score	Factors
🔭 Scope	8/20	TextRange.jsx receives a one-line class-map addition; s2.css gains two rule blocks (34 lines); sourceTextPage.ts gains setBiLayout, getSegmentSpanLayout, and isInstructionRendered; siddur-instruction-rubrics.spec.ts is a new file with 113 lines. All four files are in the reader subsystem with no new API endpoints or database changes.
🏗️ Architecture	4/20	SourceTextPage gains two new public methods and one private getter. No new module boundary is introduced and no existing dependency changes. The JS change wires an already-computed boolean (hasNoPrimarry) into the segment class map.
⚙️ Implementation	9/20	getSegmentSpanLayout uses a single page.evaluate() call to read display, float, width, direction, className, and hasInstruction atomically for both .primary and .translation spans, returning segmentWidth from getBoundingClientRect() as a denominator. isInstructionRendered uses offsetParent !== null && rect.height > 0 rather than Playwright's :visible to detect display:none ancestors. The CSS uses :has(> i.instruction) on the source-only rule and .noPrimary as the gate on the bilingual rules, with the spec comment explaining why .noPrimary is the correct trigger rather than :has(i.instruction) alone.
⚠️ Risk	5/20	The CSS selectors are narrow (.segment.noPrimary gate prevents the mixed-segment regression) and the change is easily reverted. s2.css is a global stylesheet so a specificity conflict with an unrelated rule is possible but the selector chain is 6 levels deep. The :has() pseudo-class requires verification against the project's browser support matrix. No auth, data pipeline, or API surface is touched.
✅ Quality	11/15	siddur-instruction-rubrics.spec.ts provides four E2E tests: SID-001 (rubric spans full width in bilingual mode), SID-002 (mixed segment keeps 50/50 split — regression guard), SID-003 (English UI shows rubric in source-only mode), SID-004 (Hebrew UI hides rubric in source-only mode). The stacked bilingual layout has no CSS rule and no test. The spec file's comment block names the production API endpoint used to verify segment data and explains the noPrimary vs :has(i.instruction) design decision.
🔒 Perf / Security	1/5	No performance or security work is present. The CSS selector depth is within normal bounds for the existing stylesheet.

Was this score accurate? 👍 Yes · 👎 No

How this was scored →

_{Scored by GitVelocity · How are scores calculated?}

[EliezerIsrael]

Red baseline captured against production

Ran both specs against SANDBOX_URL=https://www.sefaria.org (staging 404s on the siddur; prod has it, with the identical contentSpan primary/translation DOM and all 8 instruction tags).

test	prod (master)	reason
SID-001	✘ fail	translation.float = left, expected none — rubric still boxed in the 50% column
SID-003	✘ fail	rendered = false — rubric hidden in source-only mode
SID-002	✓ pass	guard: mixed segment (Tefillin 11) keeps its 50/50 split
SID-004	✓ pass	guard: Hebrew UI keeps the English rubric hidden

Both new-behavior tests fail for the intended reason and both guards pass, so a green run can't come from the page simply failing to load.

Green run against a local instance on this branch is pending — the local Mongo dump predates the siddur import.

One extra commit

2fb203be9 fixes SourceTextPage.setContentLanguage, which targeted getByRole('button', { name: 'Toggle Reader Menu Display Settings' }) — that matches nothing on current master. The toggle is a div whose accessible name is now common.text_display_options, and it had no callers, so it had gone stale unnoticed. Both it and the layout radios are also interface-language dependent, so the selectors now anchor on the hardcoded .readerOptionsTooltip class and the radios' English value attributes per CLAUDE.md §2 rule 15.

[EliezerIsrael]

✅ Green run — verified end to end

Ran against a local instance on this branch (webpack client build, siddur data restored from the prod dump, redis TOC cache cleared so the siddur appears in the TOC).

test	master (prod)	this branch (local)
SID-001 — rubric spans both columns	✘ float: left	✓ pass
SID-003 — rubric shown in source-only + EN UI	✘ rendered: false	✓ pass
SID-002 — mixed segment keeps 50/50 (guard)	✓	✓ pass
SID-004 — Hebrew UI keeps rubric hidden (guard)	✓	✓ pass

Both behaviors flip red→green, and both guards hold on each side. Confirmed in the DOM that 6 of the 16 segments in Weekdays, Tefillin pick up .noPrimary, matching the 6 rubric-only segments predicted from the API sweep.

Ready for review.