fix(viewer): bound chronicle a11y footprint + suppress inter-beat transition meta-text (#752) - #890
Conversation
…nsition meta-text (#752) The 2026-06-15 confirm sweep flagged #752 MAJOR by 3 of 5 personas (newbie, adversarial, narrative). Verbatim: "Chronicle log grows into one massive block — later beats invisible in a11y tree" / "Oversized chronicle log pushes Actions section out of a11y tree entirely" / "buries action buttons — player can't tell if DM is done" / "hiding all action controls from screen reader". Plus adversarial: "DUPLICATE DM narration for the opening beat" and "Engine META-TEXT transition phrases LEAK into the chronicle between player beats". FELT MECHANISM: the screen-reader / QA blind-player reads the page via a LINEAR, length-capped accessibility snapshot (qa/playwright/palette_server.js does ariaSnapshot().slice(0,9000)) rendered in DOM order. The Chronicle (role="log") renders BEFORE the action palette + composer, so a long run of multi-paragraph DM beats fills the whole snapshot budget and the action controls are sliced off the snapshot ENTIRELY. #402 anchored the bar VISUALLY (a sticky DOM sibling) but a linear capped snapshot can't be fixed by visual positioning. FIX (viewer-only, READ-ONLY — engine stays sole writer): 1. a11y bound: cap the chronicle's ACCESSIBILITY footprint independently of its visual row count — only the most-recent CHRONICLE_A11Y_TAIL (8) rendered rows stay in the a11y tree; older rendered rows are aria-hidden (still fully VISIBLE for sighted scroll-back, full history in the Quest Journal). The latest beat is always exposed; the bound only engages once the rendered list exceeds the tail. Plus a named role="region" aria-label="Actions" so AT can target the controls. New pure exported chronicleRowAriaHidden(i,total) + CHRONICLE_A11Y_TAIL. 2. opening-beat / player-echo DEDUP: pinned the exactly-once contract (the existing #727/#740/#405 dedup already collapses the opening + the Continue/free-text echo; tests lock it so it can't regress). 3. META-TEXT leak: new high-confidence _BEAT_TRANSITION pattern in sanitizeNarration suppresses inter-beat transition stage-directions ("Moving on to the next beat", "Transitioning to the next scene", "Scene transition", "End of beat", "Beginning the next beat", "between the beats", "move to the next part of the story") while preserving real fiction ("heart skips a beat", "tavern scene", "smooth transition from the parapet to the rope", "between the beats of the drum"). Tests (JSX harness, real .jsx transpiled under Node): - viewer/tests/test_chronicle_a11y_bound.py (3): tail constant tight; action controls stay in the a11y tree after 30 rows (only the tail exposed); short chronicle exposes every row. - viewer/tests/test_chronicle_dedup_opening.py (13): opening once across bands; Continue/free-text no double-echo; wrapper + inter-beat transition meta-text suppressed; surrounding real prose preserved; in-world beat/scene/transition prose survives. fast_gate.sh PASS (215 engine). Full viewer suite 596 passed / 1 skipped.
|
Warning Review limit reached
More reviews will be available in 1 hour, 29 minutes, and 25 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8262e92913
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // exceeds the tail (a short early-session chronicle exposes every row). Standard long-log a11y | ||
| // pattern (virtualized chat transcripts do the same). Viewer stays READ-ONLY — this is purely how the | ||
| // already-rendered, engine-authored prose is exposed to assistive tech. | ||
| const CHRONICLE_A11Y_TAIL = 8; |
There was a problem hiding this comment.
In sessions where the newest DM beat(s) are multi-paragraph, exposing the last 8 chronicle rows can still exceed the blind-player snapshot's hard ariaSnapshot().slice(0, 9000) cap before the Actions region is reached. The server preserves full recent-event text up to 64 KiB per row, so a single long latest row or several normal long rows will leave the action controls sliced out even though older rows are aria-hidden; this is exactly the failure this change is meant to prevent. Please bound the exposed chronicle by character budget/summary as well as row count, or ensure Actions precede the unbounded log in the a11y tree.
Useful? React with 👍 / 👎.
…tion (review fix)
Adversarial review found the meta-text regex silently deleted real fiction sentences ('the end
of the act left them breathless', 'by the close of the scene, three lay dead', 'beginning the act
of contrition'). Root: the end-of/beginning-the arms used act/chapter/part (real-fiction words)
+ an optional forward qualifier, so they matched descriptive prose. FIX: those two arms now use
only the engine's own struct terms (beat|scene) AND require the phrase to be TERMINAL (the meta
note IS the short sentence, ending on the struct) — descriptive fiction embeds the phrase
mid-sentence with trailing prose, so it's spared. The leak cases ('End of beat.', 'Beginning the
next beat.') still suppress. +5 survive-case tests (the exact over-strip examples). Story quality
is the north star — no silent fiction deletion.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 261ca0c93f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // exceeds the tail (a short early-session chronicle exposes every row). Standard long-log a11y | ||
| // pattern (virtualized chat transcripts do the same). Viewer stays READ-ONLY — this is purely how the | ||
| // already-rendered, engine-authored prose is exposed to assistive tech. | ||
| const CHRONICLE_A11Y_TAIL = 8; |
There was a problem hiding this comment.
Fresh evidence in this revision is that CHRONICLE_A11Y_TAIL still exposes 8 complete rows with no character budget, while the blind-player path caps ariaSnapshot() at 9000 chars (qa/playwright/palette_server.js:239-245) and recentEvents rows can carry up to 64 KiB (viewer/server.py:1653-1691). In any session where the latest beat is very long, or the exposed tail contains several multi-KB DM beats, the chronicle can still consume the entire snapshot before the Actions region, so the #752 failure remains; please truncate/summarize the exposed text by character budget or place Actions before the log in the a11y tree.
Useful? React with 👍 / 👎.
| "\\b(?:begin(?:ning)?|start(?:ing)?) the (?:next |following )?" + _STRUCT_FWD + "\\s*[.!?]?\\s*$|" + | ||
| // "(time passes) between the beats/scenes" — the interstitial seam phrasing. NOT "between the | ||
| // beats OF the drum" (real fiction): the meta form is terminal, so forbid a following " of ". | ||
| "\\bbetween (?:the |these )?(?:beats|scenes)\\b(?!\\s+of\\b)|" + |
There was a problem hiding this comment.
When in-world narration uses “between the beats” as timing, this new arm matches anywhere in the sentence despite the comment saying the meta form is terminal; _stripScaffoldingSentences then drops the whole sentence. For example, You count the space between the beats, then run. sanitizes to an empty string, so legitimate player-facing narration is removed whenever beats/scenes are used as real fiction rather than wrapper meta; require the terminal meta form or a stronger prefix such as “time passes”.
Useful? React with 👍 / 👎.
…eats never slice off the action controls (#752) (#902) #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. Co-authored-by: Eva <arncalso@gmail.com>
1 participant
#752 — the chronicle must not flood the a11y tree and bury the action controls
Flagged MAJOR by 3 of 5 personas (newbie, adversarial, narrative) in the 2026-06-15 confirm sweep. Verbatim symptoms:
Felt mechanism (the why)
The screen-reader / QA blind-player reads the page via a linear, length-capped accessibility snapshot —
qa/playwright/palette_server.jsdoesariaSnapshot().slice(0, 9000)— rendered in DOM order. The Chronicle (role="log") renders before the action palette + the move composer, so a long run of multi-paragraph DM beats fills the entire 9000-char snapshot budget and the action controls get sliced off the snapshot entirely — the felt "I can't find the buttons / can't tell the DM is done." #402 anchored the action bar visually (a sticky DOM sibling), but a linear, length-capped snapshot can't be fixed by visual positioning.Fix (viewer-only, READ-ONLY — engine stays sole writer)
CHRONICLE_A11Y_TAIL(8) rendered rows stay in the accessibility tree; older rendered rows arearia-hidden(still fully visible for sighted scroll-back; full history preserved in the Quest Journal, already named by the fix(viewer): keep latest chronicle beat + action box visible; camp-rest feedback when DM busy #402 "earlier beats are in your Quest Journal" summary). The latest beat is always exposed; the bound only engages once the rendered list exceeds the tail. Plus a namedrole="region" aria-label="Actions"so AT can target the controls directly. New pure exportedchronicleRowAriaHidden(i, total)+CHRONICLE_A11Y_TAIL./eventsbands) and theContinue/free-text echo (optimistic vs/chatreplay). Tests now lock the exactly-once contract so the GUI-path can't regress._BEAT_TRANSITIONpattern insanitizeNarrationsuppresses inter-beat transition stage-directions ("Moving on to the next beat", "Transitioning to the next scene", "Scene transition", "End of beat", "Beginning the next beat", "between the beats", "we now move to the next part of the story") while preserving real fiction ("heart skips a beat", "the tavern scene", "a smooth transition from the parapet to the rope", "between the beats of the drum").Invariants honored
dangerouslySetInnerHTML;sanitizeNarrationpreserved (extended additively).Tests (JSX harness — the real
.jsxtranspiled under Node, no reimplementation)viewer/tests/test_chronicle_a11y_bound.py(3): tail constant is tight; action controls stay in the a11y tree after 30 chronicle rows (only the tail exposed, newest never hidden, oldest hidden); a short chronicle exposes every row.viewer/tests/test_chronicle_dedup_opening.py(13): opening DM narration appears once across the recent + live bands;Continue/free-text actions don't double-echo (no rawYou"continue"dialog template); wrapper progress + inter-beat transition meta-text suppressed; surrounding real prose preserved; in-worldbeat/scene/transitionprose survives.Verification
bash qa/fast_gate.sh→ PASS (215 deterministic engine + seat-path).test_wrapper_progress_sync.pystill green (wrapper-progress constants untouched).DO NOT MERGE — for review.