refactor(teleprompter): enforce each invariant in one place

A maintenance pass over the view. Nothing changes on screen.

The position was bounded twice on the playing path: advance() clamped its
result and the loop clamped it again. The loop has to clamp regardless, for
nudges and for a document which shrank, so advance() now reports reaching
the end and leaves the bound to its only caller.

maxScroll was floored at zero at each of four reads, though measure() is its
only writer and already floors it. The invariant is stated once, where it is
established.

Arrival at an eased target was tested twice: easeCatchUp snaps to the target
once it is close enough, and hasArrived then measured the same distance
again. Equality with the target is the signal, so hasArrived is gone.

Speed was clamped to its range by the option parser and then clamped to the
identical range again by clampSpeed, on the same call. Only the keys and
buttons, which add deltas, still need to clamp.

The Enter guard in the key handler could never fire: the keymap does not bind
Enter, so the event was already passing through untouched. The reason Enter
stays unbound now lives in the keymap, with a test, rather than as a branch
which reads as though something was being prevented.

Also drops a redundant early return in ReadingLine, an export with no callers
outside its module, and a JSON round trip in the param mirror which existed
only to satisfy a dependency array.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Cb8RVPNQ2ETPJxdy4b8CHf

apps/client/src/views/teleprompter/Teleprompter.tsx

@@ -59,15 +59,13 @@ function Teleprompter({ rundown, rundownMetadata, customFields }: TeleprompterDa
    * frame of the previous orientation paint first, which on a prompter reads as
    * a flash. The speed option is kept live the same way, in useTeleprompterScroll.
    */
-  const [live, setLive] = useState({ flipH: options.flipH, flipV: options.flipV, fontSize: options.fontSize });
-  const [liveFromParams, setLiveFromParams] = useState(live);
-  if (
-    liveFromParams.flipH !== options.flipH ||
-    liveFromParams.flipV !== options.flipV ||
-    liveFromParams.fontSize !== options.fontSize
-  ) {
-    const fromParams = { flipH: options.flipH, flipV: options.flipV, fontSize: options.fontSize };
-    setLiveFromParams(fromParams);
+  const fromParams = { flipH: options.flipH, flipV: options.flipV, fontSize: options.fontSize };
+  const paramsKey = `${fromParams.flipH}|${fromParams.flipV}|${fromParams.fontSize}`;
+
+  const [live, setLive] = useState(fromParams);
+  const [seededFrom, setSeededFrom] = useState(paramsKey);
+  if (seededFrom !== paramsKey) {
+    setSeededFrom(paramsKey);
     setLive(fromParams);
   }
 
@@ -106,6 +104,14 @@ function Teleprompter({ rundown, rundownMetadata, customFields }: TeleprompterDa
 
   const handleFontSize = (steps: number) =>
     setLive((current) => ({ ...current, fontSize: stepFontSize(current.fontSize, steps) }));
+
+  /**
+   * Back to the view's default rather than to the size this client happened to
+   * open at. Now that the keys write the size into the query there is no longer
+   * a separate configured value to return to, and remembering one privately
+   * would mean two people on the same link getting different results from the
+   * same key.
+   */
   const handleResetFontSize = () => setLive((current) => ({ ...current, fontSize: defaults.fontSize }));
   const handleToggleHelp = () => setShowHelp((current) => !current);
 

apps/client/src/views/teleprompter/__tests__/teleprompter.keymap.test.ts

@@ -95,4 +95,10 @@ describe('resolveTeleprompterAction()', () => {
   test('ignores keys it does not bind', () => {
     expect(resolveTeleprompterAction(makeEvent({ code: 'KeyQ', key: 'q' }))).toBeNull();
   });
+
+  test('leaves Enter alone, so a focused transport button can still be pressed', () => {
+    // Space belongs to the prompter wherever focus is, which leaves Enter as the
+    // only way to work the overlay from the keyboard
+    expect(resolveTeleprompterAction(makeEvent({ code: 'Enter', key: 'Enter' }))).toBeNull();
+  });
 });

apps/client/src/views/teleprompter/__tests__/teleprompter.scroll.test.ts

@@ -76,16 +76,17 @@ describe('advance()', () => {
     expect(position).toBeCloseTo((pxPerSecond * 100) / 60, 5);
   });
 
-  test('clamps at the bounds of the document', () => {
-    expect(advance(0, -100, 1, 500).position).toBe(0);
-    expect(advance(490, 100, 1, 500).position).toBe(500);
-  });
-
   test('reports the end of the script once the bottom is reached', () => {
     expect(advance(499, 100, 1, 500).atEnd).toBe(true);
     expect(advance(100, 100, 1, 500).atEnd).toBe(false);
   });
 
+  test('reports the end without bounding the position, which the caller owns', () => {
+    // the loop has to bound the position anyway, for nudges and for a document
+    // which shrank, so this does not do it a second time
+    expect(advance(490, 100, 1, 500)).toEqual({ position: 590, atEnd: true });
+  });
+
   test('never reports the end for a document which does not overflow', () => {
     // it may simply not have been measured yet, and stopping playback on an
     // unmeasured document would look like the prompter refusing to run

apps/client/src/views/teleprompter/reading-line/ReadingLine.tsx

@@ -14,10 +14,6 @@ interface ReadingLineProps {
  * pointing at a position between two of them.
  */
 export default function ReadingLine({ showReadingLine, dimPast }: ReadingLineProps) {
-  if (!showReadingLine && !dimPast) {
-    return null;
-  }
-
   return (
     <>
       {dimPast && <div className='teleprompter__dim' />}

apps/client/src/views/teleprompter/teleprompter.keymap.ts

@@ -23,6 +23,11 @@ export type TeleprompterKeyEvent = {
  * hardware protocol: foot pedals and hand controllers are USB HID devices which
  * emit these very keystrokes, so matching it is what makes them work here.
  *
+ * Enter is deliberately left unbound. Space is the run/stop pedal and has to
+ * reach the prompter wherever focus happens to be, so the transport overlay
+ * needs one key which still activates whichever control is focused, and Enter
+ * is it. Binding it here would take the overlay's keyboard operation away.
+ *
  * @returns the action to run, or null when the view should ignore the event
  */
 export function resolveTeleprompterAction(event: TeleprompterKeyEvent): TeleprompterAction | null {

apps/client/src/views/teleprompter/teleprompter.options.ts

@@ -7,7 +7,7 @@ import type { ViewOption } from '../../common/components/view-params-editor/view
 import { makeOptionsFromCustomFields } from '../../common/components/view-params-editor/viewParams.utils';
 import { PresetContext } from '../../common/context/PresetContext';
 import { isStringBoolean } from '../common/viewUtils';
-import { clampSpeed, DEFAULT_SPEED, MAX_FONT_SIZE, MAX_SPEED, MIN_FONT_SIZE, MIN_SPEED } from './teleprompter.scroll';
+import { DEFAULT_SPEED, MAX_FONT_SIZE, MAX_SPEED, MIN_FONT_SIZE, MIN_SPEED } from './teleprompter.scroll';
 import type { HeadingSource, TeleprompterOptions } from './teleprompter.types';
 
 /**
@@ -230,7 +230,7 @@ export function getOptionsFromParams(
     hideEmpty: toBoolean(getValue('hideEmpty'), defaults.hideEmpty),
     showGroups: toBoolean(getValue('showGroups'), defaults.showGroups),
 
-    speed: clampSpeed(toNumber(getValue('speed'), bounds.speed, defaults.speed)),
+    speed: toNumber(getValue('speed'), bounds.speed, defaults.speed),
     followLoaded: toBoolean(getValue('followLoaded'), defaults.followLoaded),
 
     fontSize: toNumber(getValue('fontSize'), bounds.fontSize, defaults.fontSize),

apps/client/src/views/teleprompter/teleprompter.scroll.ts

@@ -35,13 +35,9 @@ export const MIN_FONT_SIZE = 12;
 export const MAX_FONT_SIZE = 400;
 const FONT_SIZE_STEP_RATIO = 1.1;
 
-export function clampFontSize(value: number): number {
-  return clamp(value, MIN_FONT_SIZE, MAX_FONT_SIZE);
-}
-
 /** @param steps how many presses to apply, negative to shrink */
 export function stepFontSize(current: number, steps: number): number {
-  return clampFontSize(Math.round(current * FONT_SIZE_STEP_RATIO ** steps));
+  return clamp(Math.round(current * FONT_SIZE_STEP_RATIO ** steps), MIN_FONT_SIZE, MAX_FONT_SIZE);
 }
 
 /**
@@ -85,11 +81,17 @@ export function frameDeltaSeconds(deltaMs: number): number {
 }
 
 /**
- * Advances the scroll position at a constant rate.
+ * Advances the scroll position at a constant rate, and reports having reached
+ * the bottom.
  *
  * The position is kept as a float by the caller: at readable prompter speeds the
  * per frame delta is well under a pixel, so rounding on every frame would stall
  * the scroll entirely. We return the exact float and let the DOM round on write.
+ *
+ * Deliberately does not bound the result. The caller has to bound the position
+ * anyway, because a nudge or a shrinking document can put it out of range on
+ * frames this never runs on, and one clamp in one place is easier to trust than
+ * the same rule applied twice on one of the paths.
  */
 export function advance(
   position: number,
@@ -97,7 +99,7 @@ export function advance(
   deltaSeconds: number,
   maxScroll: number,
 ): { position: number; atEnd: boolean } {
-  const next = clamp(position + pxPerSecond * deltaSeconds, 0, Math.max(maxScroll, 0));
+  const next = position + pxPerSecond * deltaSeconds;
   // a document which does not overflow is never "at the end": it may simply not
   // have been measured yet, and stopping playback on that would be wrong
   return { position: next, atEnd: maxScroll > 0 && next >= maxScroll };
@@ -108,13 +110,12 @@ export function advance(
  *
  * Framerate independent: the same wall clock duration produces the same curve
  * regardless of how many frames it was sampled over.
+ *
+ * Snaps to the target once it is close enough rather than approaching it
+ * forever, so the caller can treat equality with the target as arrival.
  */
 export function easeCatchUp(current: number, target: number, deltaSeconds: number): number {
   if (deltaSeconds <= 0) return current;
   const next = target + (current - target) * Math.exp(-CATCH_UP_RATE * deltaSeconds);
   return Math.abs(next - target) < CATCH_UP_EPSILON ? target : next;
 }
-
-export function hasArrived(current: number, target: number): boolean {
-  return Math.abs(current - target) < CATCH_UP_EPSILON;
-}

apps/client/src/views/teleprompter/useMirrorLiveParams.ts

@@ -1,4 +1,4 @@
-import { use, useEffect } from 'react';
+import { use, useEffect, useRef } from 'react';
 import { useSearchParams } from 'react-router';
 
 import { PresetContext } from '../../common/context/PresetContext';
@@ -35,8 +35,20 @@ export function useMirrorLiveParams(live: Record<string, string | null>) {
   const [, setSearchParams] = useSearchParams();
   const isPreset = Boolean(use(PresetContext));
 
-  // the effect depends on the values rather than the object, which is new every render
+  /**
+   * The caller passes a fresh object every render, so the effect keys off the
+   * values instead. Depending on the object itself would restart the timer on
+   * every unrelated re-render, and the view re-renders on socket traffic, so the
+   * write would keep being postponed and never land.
+   *
+   * The values are then read through a ref rather than closed over, which keeps
+   * the timeout reading the latest of them and keeps the dependency honest.
+   */
   const serialised = JSON.stringify(live);
+  const liveRef = useRef(live);
+  useEffect(() => {
+    liveRef.current = live;
+  });
 
   useEffect(() => {
     /**
@@ -50,7 +62,7 @@ export function useMirrorLiveParams(live: Record<string, string | null>) {
       setSearchParams(
         (current) => {
           const next = new URLSearchParams(current);
-          for (const [key, value] of Object.entries(JSON.parse(serialised) as Record<string, string | null>)) {
+          for (const [key, value] of Object.entries(liveRef.current)) {
             if (value === null) {
               next.delete(key);
             } else {

apps/client/src/views/teleprompter/useTeleprompterControls.ts

@@ -74,17 +74,6 @@ export function useTeleprompterControls(args: UseTeleprompterControlsArgs) {
         return;
       }
 
-      /**
-       * Enter is left to a focused control so the overlay stays operable from
-       * the keyboard. Space is not: it is the play/pause pedal, and a prompter
-       * which stops rolling because the operator last touched a button would be
-       * broken in the one moment it matters. The overlay drops focus after a
-       * click so the two never compete for the same press.
-       */
-      if (event.key === 'Enter' && target?.closest('button, a, [role="button"]')) {
-        return;
-      }
-
       const action = resolveTeleprompterAction(event);
       if (!action) return;
 

apps/client/src/views/teleprompter/useTeleprompterScroll.ts

@@ -7,7 +7,6 @@ import {
   clampSpeed,
   easeCatchUp,
   frameDeltaSeconds,
-  hasArrived,
   linesPerMinuteToPxPerSecond,
 } from './teleprompter.scroll';
 import type { ScriptBlock, TeleprompterController } from './teleprompter.types';
@@ -83,6 +82,7 @@ export function useTeleprompterScroll({
   const runningRef = useRef(false);
   const speedRef = useRef(initialSpeed);
   const lineHeightRef = useRef(0);
+  /** never negative: measure() is the only writer and floors it at zero */
   const maxScrollRef = useRef(0);
   const catchUpTargetRef = useRef<number | null>(null);
   const pendingDeltaRef = useRef(0);
@@ -136,8 +136,9 @@ export function useTeleprompterScroll({
 
     if (catchUpTargetRef.current !== null) {
       next = easeCatchUp(next, catchUpTargetRef.current, deltaSeconds);
-      if (hasArrived(next, catchUpTargetRef.current)) {
-        next = catchUpTargetRef.current;
+      // easeCatchUp snaps to the target once it is close enough, so landing on it
+      // exactly is the arrival signal and needs no second distance test
+      if (next === catchUpTargetRef.current) {
         catchUpTargetRef.current = null;
       }
     } else if (runningRef.current) {
@@ -151,7 +152,8 @@ export function useTeleprompterScroll({
       }
     }
 
-    const clamped = clamp(next, 0, Math.max(maxScrollRef.current, 0));
+    // the one place the position is bounded, whichever branch produced it
+    const clamped = clamp(next, 0, maxScrollRef.current);
     // touch the DOM only when the position actually moved, so a paused prompter
     // costs arithmetic rather than a scroll write on every frame
     if (clamped !== posRef.current) {
@@ -203,11 +205,12 @@ export function useTeleprompterScroll({
 
   // the configured speed seeds the live one, and reclaims it whenever the option
   // changes. Adjusted during render rather than in an effect so the readout never
-  // commits the stale value first
+  // commits the stale value first. It arrives already bounded by the option parser,
+  // so only the keys and buttons, which add deltas, have to clamp
   const [speedFromOption, setSpeedFromOption] = useState(initialSpeed);
   if (speedFromOption !== initialSpeed) {
     setSpeedFromOption(initialSpeed);
-    setSpeed(clampSpeed(initialSpeed));
+    setSpeed(initialSpeed);
   }
 
   // the observer covers every reflow that matters, script edits included, so
@@ -269,7 +272,7 @@ export function useTeleprompterScroll({
     const offset = (scroller.clientHeight * readingLinePos) / 100;
     const top = target.getBoundingClientRect().top - scroller.getBoundingClientRect().top + scroller.scrollTop - offset;
 
-    catchUpTargetRef.current = clamp(top, 0, Math.max(maxScrollRef.current, 0));
+    catchUpTargetRef.current = clamp(top, 0, maxScrollRef.current);
     setAtEnd(false);
   }, [selectedEventId, followLoaded, followLocked, readingLinePos, hasSelectedBlock]);
 
@@ -325,7 +328,7 @@ export function useTeleprompterScroll({
         const scroller = scrollerRef.current;
         if (!scroller) return;
         const distance = scroller.clientHeight * PAGE_FRACTION * direction;
-        catchUpTargetRef.current = clamp(posRef.current + distance, 0, Math.max(maxScrollRef.current, 0));
+        catchUpTargetRef.current = clamp(posRef.current + distance, 0, maxScrollRef.current);
         setAtEnd(false);
       },
       changeSpeed: (delta: number) => setSpeed((current) => clampSpeed(current + delta)),
@@ -335,7 +338,7 @@ export function useTeleprompterScroll({
         setAtEnd(false);
       },
       jumpToEnd: () => {
-        catchUpTargetRef.current = Math.max(maxScrollRef.current, 0);
+        catchUpTargetRef.current = maxScrollRef.current;
         // the eased branch never reports the end, only the playing one does
         if (maxScrollRef.current > 0) {
           runningRef.current = false;