From 9efd6bc7bfbbbf91d842f1ebb59858d0155c6048 Mon Sep 17 00:00:00 2001 From: Eva Date: Mon, 15 Jun 2026 11:26:14 +0700 Subject: [PATCH] fix(viewer): bound chronicle a11y by cumulative char budget so long beats never slice off the action controls (#752) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #890 bounded the chronicle's a11y exposure to the most-recent CHRONICLE_A11Y_TAIL=8 rendered rows (older rows aria-hidden), but a ROW count is not a CHAR count. A normal mid-session DM beat is a multi-paragraph turn (~1000–2000 chars), so 8 exposed rows still total ~12000+ chars of a11y YAML — past the 9000-char ariaSnapshot().slice(0,9000) budget the QA blind-player reads. Because the Chronicle (role="log") renders BEFORE the Actions palette + composer in DOM order, that overflow STILL slices the action controls off the snapshot entirely ("player can't find the action buttons") — which is why #752 remained flagged by 2-3/5 personas at sweep 3582dc2 DESPITE #890. Fix (additive, viewer-only, READ-ONLY on state): under the CHRONICLE_A11Y_TAIL row ceiling, ALSO bound the EXPOSED (non-aria-hidden) rows by a cumulative CHARACTER budget. • new const CHRONICLE_A11Y_CHAR_BUDGET = 2800 — conservative; leaves headroom under 9000 for the YAML inflation + the Actions palette + composer that follow in the DOM. • new pure, window-exported helper chronicleA11yExposedCount(visibleTextLengths): accumulates visible text length from the NEWEST row backward, stops before exceeding the budget, ALWAYS exposes ≥1 (the newest beat is never hidden, even one bigger than the whole budget), never more than CHRONICLE_A11Y_TAIL. • chronicleRowVisibleTextLength derives each row's visible text length the same way LogEntry renders it (sanitizeNarration for narration/dialogue, raw text/detail else). • chronicleRowAriaHidden gains an optional `exposed` cutoff; the 2-arg row-only form still works (legacy callers keep the original tail bound). No regression for an early-session player: a SHORT session (cumulative text under budget AND rows ≤ tail) still exposes EVERY row. The newest beat is ALWAYS exposed. Tests: extend viewer/tests/test_chronicle_a11y_bound.py with a long-beat case (8 rows × ~1500 chars must expose fewer than 8 so cumulative exposed text ≤ budget), the always-expose-newest invariant, the conservative-budget bound, and the short-session no-regression case. Existing tail-constant + row-bound tests unchanged and still green. --- viewer/openworlds/screen-table.jsx | 88 ++++++++++++++++++++--- viewer/tests/test_chronicle_a11y_bound.py | 86 ++++++++++++++++++++++ 2 files changed, 164 insertions(+), 10 deletions(-) diff --git a/viewer/openworlds/screen-table.jsx b/viewer/openworlds/screen-table.jsx index 6e6dc94a..5b459c8e 100644 --- a/viewer/openworlds/screen-table.jsx +++ b/viewer/openworlds/screen-table.jsx @@ -267,6 +267,22 @@ function lastVisibleChronicleIndex(rows) { return -1; } +// #752: the VISIBLE text length a chronicle row contributes to the accessibility snapshot — the same +// prose LogEntry renders for that row. narration/dialogue flow through sanitizeNarration (the player- +// facing filter); every other kind (action/dialog/roll/fallback) renders its raw `text`/`detail`. +// Used to bound the EXPOSED rows by a cumulative char budget (chronicleA11yExposedCount), so a long +// run of multi-paragraph DM beats can't overflow the snapshot and slice off the action controls. +// Pure (no DOM) so the char bound stays unit-testable. +function chronicleRowVisibleTextLength(entry) { + if (!entry) return 0; + const kind = entry.kind || "narration"; + if (kind === "narration" || kind === "dialogue") { + return (sanitizeNarration(entry.text) || "").length; + } + const text = entry.text || entry.detail || ""; + return String(text).length; +} + // #337: the quick-action buttons (Continue / Say / Do / Check / Save) and the dice buttons are // icon+label only — a first-timer can't tell how they differ from typing free-text + Declare, so // the #324 newbie ignored all of them. These short hints surface as native `title=` tooltips @@ -359,19 +375,63 @@ const CHRONICLE_RENDER_CAP = 50; // already-rendered, engine-authored prose is exposed to assistive tech. const CHRONICLE_A11Y_TAIL = 8; +// #752 (STILL flagged at sweep 3582dc2 DESPITE the row-count tail above): a ROW count is not a CHAR +// count. CHRONICLE_A11Y_TAIL=8 bounds the EXPOSED rows to 8, but a normal mid-session beat is a +// multi-paragraph DM turn (~1000–2000 chars), so 8 exposed rows still total ~12000+ chars of a11y +// YAML — past the 9000-char `ariaSnapshot().slice(0, 9000)` budget the reader (and the QA +// blind-player) caps at. The Chronicle (`role="log"`) renders BEFORE the Actions palette + composer, +// so that overflow STILL slices the action controls off the snapshot entirely ("player can't find +// the action buttons"). The tail row-count does not bound the CHARACTER footprint. So under the row +// ceiling we ALSO bound the exposed rows by a cumulative CHARACTER budget: a conservative cap that +// leaves headroom under 9000 for the YAML inflation + the Actions palette + composer that follow. +const CHRONICLE_A11Y_CHAR_BUDGET = 2800; + +// How many of the most-recent rendered rows to EXPOSE to the accessibility tree, given each rendered +// row's VISIBLE text length (most-recent LAST, mirroring renderedLog order). Pure + exported so the +// char bound is unit-testable without mounting the component (mirrors chronicleRowAriaHidden / +// buildChronicleLog). Accumulate text length from the NEWEST row backward; stop before exceeding the +// budget; ALWAYS expose at least 1 (the newest beat — the player's most recent DM reply — is never +// hidden, even a single beat larger than the whole budget); never expose more than the row tail. A +// short, low-volume session (cumulative text under budget AND rows ≤ tail) still exposes EVERY row. +function chronicleA11yExposedCount(visibleTextLengths) { + const lens = Array.isArray(visibleTextLengths) ? visibleTextLengths : []; + const n = lens.length; + if (n === 0) return 0; + const ceiling = Math.min(n, CHRONICLE_A11Y_TAIL); + let exposed = 0; + let cumulative = 0; + for (let k = 0; k < ceiling; k += 1) { + const len = Number(lens[n - 1 - k]) || 0; // walk newest → older + // Always expose the newest beat (k===0) regardless of its size; otherwise stop before the + // budget would be exceeded so the action controls stay inside the snapshot. + if (k > 0 && cumulative + len > CHRONICLE_A11Y_CHAR_BUDGET) break; + cumulative += len; + exposed += 1; + } + return exposed; +} + // True when the chronicle row at index `i` of `total` rendered rows must be `aria-hidden` — i.e. it -// is OLDER than the most-recent CHRONICLE_A11Y_TAIL rows. Pure + exported so the bound is +// falls OUTSIDE the most-recent EXPOSED window. `exposed` is the char-aware cutoff from +// chronicleA11yExposedCount (defaults to the CHRONICLE_A11Y_TAIL row count for legacy callers that +// only know the row total, preserving the original row-only bound). Pure + exported so the bound is // unit-testable without mounting the component (mirrors buildChronicleLog / computePlayGate). When -// the rendered list is at/under the tail, NOTHING is hidden (every beat announced). -function chronicleRowAriaHidden(i, total) { +// the exposed window covers the whole list, NOTHING is hidden (every beat announced). +function chronicleRowAriaHidden(i, total, exposed) { const n = Number(total) || 0; const idx = Number(i); - if (!Number.isFinite(idx) || n <= CHRONICLE_A11Y_TAIL) return false; - return idx < n - CHRONICLE_A11Y_TAIL; + if (!Number.isFinite(idx)) return false; + const window_ = Number.isFinite(Number(exposed)) + ? Math.max(1, Math.min(n, Number(exposed))) + : Math.min(n, CHRONICLE_A11Y_TAIL); + if (n <= window_) return false; + return idx < n - window_; } if (typeof window !== "undefined") { window.CHRONICLE_RENDER_CAP = CHRONICLE_RENDER_CAP; window.CHRONICLE_A11Y_TAIL = CHRONICLE_A11Y_TAIL; + window.CHRONICLE_A11Y_CHAR_BUDGET = CHRONICLE_A11Y_CHAR_BUDGET; + window.chronicleA11yExposedCount = chronicleA11yExposedCount; window.chronicleRowAriaHidden = chronicleRowAriaHidden; } @@ -700,6 +760,12 @@ function ScreenTable({ onNavigate, state, setState, liveSession }) { const hiddenLogCount = Math.max(0, visibleLog.length - CHRONICLE_RENDER_CAP); const renderedLog = hiddenLogCount > 0 ? visibleLog.slice(visibleLog.length - CHRONICLE_RENDER_CAP) : visibleLog; const lastVisibleLogIndex = lastVisibleChronicleIndex(renderedLog); + // #752: bound the chronicle's a11y footprint by a cumulative CHARACTER budget (not just the row + // tail) — long multi-paragraph DM beats can fill the row tail with ~12000 chars and STILL slice + // the action controls off the 9000-char ariaSnapshot. chronicleA11yExposedCount walks the rendered + // rows' visible text lengths (most-recent last) and returns how many trailing rows stay exposed; + // the rest are aria-hidden (still visible for sighted scroll-back / preserved in the Quest Journal). + const chronicleA11yExposed = chronicleA11yExposedCount(renderedLog.map(chronicleRowVisibleTextLength)); const actionById = (id) => actions.find((a) => a.id === id); const enabledActionById = (id) => enabledActions.find((a) => a.id === id); const composerMode = COMPOSER_MODES[composerModeId] || COMPOSER_MODES.do; @@ -1227,11 +1293,13 @@ function ScreenTable({ onNavigate, state, setState, liveSession }) { ref={i === lastVisibleLogIndex ? latestBeatRef : null} data-worldos-testid={i === lastVisibleLogIndex ? "chronicle-latest-beat" : undefined} // #752: older rendered rows are aria-hidden so the chronicle's accessibility subtree - // stays bounded (the most-recent CHRONICLE_A11Y_TAIL rows only) and the action - // controls below are never sliced off the (length-capped) a11y snapshot. They remain - // fully VISIBLE for sighted scroll-back, and the full history is in the Quest Journal. - // The latest beat is never hidden (chronicleRowAriaHidden keeps the tail exposed). - aria-hidden={chronicleRowAriaHidden(i, renderedLog.length) ? "true" : undefined} + // stays bounded and the action controls below are never sliced off the (length-capped) + // a11y snapshot. The exposed window is the most-recent rows whose CUMULATIVE visible + // text fits CHRONICLE_A11Y_CHAR_BUDGET (under the CHRONICLE_A11Y_TAIL row ceiling) — a + // ROW count alone can't bound the CHAR footprint that overflows the snapshot. Hidden + // rows stay fully VISIBLE for sighted scroll-back; the full history is in the Quest + // Journal. The latest beat is never hidden (chronicleA11yExposedCount keeps it exposed). + aria-hidden={chronicleRowAriaHidden(i, renderedLog.length, chronicleA11yExposed) ? "true" : undefined} style={{ scrollMarginBlock: 12 }} > diff --git a/viewer/tests/test_chronicle_a11y_bound.py b/viewer/tests/test_chronicle_a11y_bound.py index 255ff84b..35d10717 100644 --- a/viewer/tests/test_chronicle_a11y_bound.py +++ b/viewer/tests/test_chronicle_a11y_bound.py @@ -137,3 +137,89 @@ def test_short_chronicle_exposes_every_row_to_a11y_tree(): "a short chronicle (≤ the a11y tail) must expose every row — the a11y bound only engages " "for a long log, never for an early-session player" ) + + +# --------------------------------------------------------------------------------------------- # +# #752 (STILL flagged at sweep 3582dc2 DESPITE the row-count bound): a ROW count is not a CHAR +# count. CHRONICLE_A11Y_TAIL=8 multi-paragraph DM beats (~1000–2000 chars each) still total +# ~12000+ chars of exposed a11y YAML — past the 9000 ariaSnapshot() budget — so the Actions / +# composer (DOM siblings AFTER the log) are STILL sliced off the snapshot. The fix bounds the +# EXPOSED (non-aria-hidden) rows by a cumulative CHARACTER budget, under the tail row ceiling: +# accumulate visible text length from the NEWEST row backward, stop before exceeding the budget, +# always expose ≥1 (the newest beat is never hidden), never more than CHRONICLE_A11Y_TAIL. +# --------------------------------------------------------------------------------------------- # +def test_char_budget_constant_is_present_and_conservative(): + # The char budget must EXIST, be window-exported, and leave real headroom under the 9000-char + # ariaSnapshot() budget for the YAML inflation + the Actions palette + composer that follow. + vals = _eval_screen_table( + "({ budget: sb.window.CHRONICLE_A11Y_CHAR_BUDGET, tail: sb.window.CHRONICLE_A11Y_TAIL })" + ) + assert isinstance(vals["budget"], int), "CHRONICLE_A11Y_CHAR_BUDGET must be exported on window" + assert 1000 <= vals["budget"] <= 5000, ( + f"char budget {vals['budget']} must be conservative — large enough for a couple of recent " + "beats, small enough to leave headroom under the 9000-char snapshot for the action controls" + ) + + +def test_long_beats_expose_fewer_rows_so_cumulative_text_fits_budget(): + # The headline #752 assertion the row-count bound MISSES: 8 rendered rows each ~1500 chars + # (a normal mid-session of multi-paragraph DM beats) must expose FEWER than 8 rows such that + # the cumulative EXPOSED text stays <= CHRONICLE_A11Y_CHAR_BUDGET — otherwise the ~12000 chars + # of exposed YAML slice the Actions/composer off the snapshot. + out = _eval_screen_table( + "(function(){" + " var n = 8, per = 1500;" + " var lens = [];" + " for (var i = 0; i < n; i++) lens.push(per);" # most-recent last (uniform here) + " var exposed = sb.window.chronicleA11yExposedCount(lens);" + " var budget = sb.window.CHRONICLE_A11Y_CHAR_BUDGET;" + " var tail = sb.window.CHRONICLE_A11Y_TAIL;" + # cumulative exposed text = the last `exposed` rows + " var cum = 0; for (var j = n - exposed; j < n; j++) cum += lens[j];" + " return { n: n, exposed: exposed, cum: cum, budget: budget, tail: tail };" + "})()" + ) + assert out["exposed"] < out["n"], ( + f"with {out['n']} long (~1500-char) beats the char bound MUST expose fewer than all " + f"{out['n']} (got {out['exposed']}) — a row-count-only bound exposes all 8 = ~12000 chars " + "and slices the action controls off the 9000-char snapshot" + ) + assert out["cum"] <= out["budget"], ( + f"cumulative exposed text {out['cum']} must stay within the {out['budget']}-char budget so " + "the Actions palette + composer land inside the snapshot" + ) + assert out["exposed"] <= out["tail"], "the char bound may shrink the exposed set but never grow it past the row tail" + + +def test_newest_beat_is_always_exposed_even_when_it_alone_exceeds_budget(): + # Invariant: the newest beat is ALWAYS announced, even a single huge beat bigger than the whole + # budget — we never hide the player's most recent DM reply. + out = _eval_screen_table( + "(function(){" + " var budget = sb.window.CHRONICLE_A11Y_CHAR_BUDGET;" + " var lens = [800, 800, budget * 3];" # newest beat alone dwarfs the budget + " var exposed = sb.window.chronicleA11yExposedCount(lens);" + " return { exposed: exposed };" + "})()" + ) + assert out["exposed"] >= 1, "the newest beat must always be exposed — never hide the latest DM reply" + + +def test_short_low_volume_session_still_exposes_every_row_no_regression(): + # No regression: a short session whose cumulative text is UNDER the budget AND whose row count + # is <= the tail still exposes EVERY row (the char bound is additive, it only shrinks a long + # high-volume log, never an early-session player's chronicle). + out = _eval_screen_table( + "(function(){" + " var tail = sb.window.CHRONICLE_A11Y_TAIL;" + " var n = Math.min(4, tail);" + " var lens = [];" + " for (var i = 0; i < n; i++) lens.push(120);" # small beats, well under budget + " var exposed = sb.window.chronicleA11yExposedCount(lens);" + " return { n: n, exposed: exposed };" + "})()" + ) + assert out["exposed"] == out["n"], ( + "a short low-volume session (cumulative text under budget AND rows <= tail) must expose " + "every row — the char bound must not regress the early-session player" + )