Skip to content

fix(viewer): Fit recorded video and scrub bar within the browser window - #6

Merged
alvarohulse merged 3 commits into
mainfrom
codex/viewer-fit-available-space
Aug 13, 2026
Merged

alvarohulse merged 3 commits into
mainfrom
codex/viewer-fit-available-space

Conversation

@alvarohulse

Copy link
Copy Markdown
Owner

Intent

Fix ProofShot viewer.html on constrained browser viewports such as Doggo's 1280x577 Browser so the entire recorded video, native controls, and ProofShot scrub bar remain visible without distorting the recorded aspect ratio or misaligning overlays. Preserve responsive mobile behavior and legacy sessions whose aspect ratio becomes known only after metadata loads. The user explicitly reported the previously merged intrinsic-dimension fix was still visibly clipped on Doggo.

What Changed

  • Reworked the viewer layout in src/artifacts/viewer.ts to a window-height flex column: body is 100vh with overflow: hidden, the header is capped at max-height: 50% with internal scrolling, and .viewer grows into the remainder via flex: 1 1 auto; min-height: 0 (replacing the fixed calc(100vh - 180px)).
  • Added a .video-fit sizing path — the container carries an aspect-ratio from the recorded viewport, and a new fitVideoToAvailableSpace() computes the fitted width from the intrinsic (or recorded) video size and applies it to .video-wrapper, so the scrub bar spans exactly the video width. It re-runs on resize, on loadedmetadata (covering legacy sessions with no recorded viewport), and on description show/hide; the ≤768px media query restores block layout, height: auto, and page scrolling.
  • Extended src/artifacts/viewer.test.ts with a styleRule() helper and assertions for the fit container markup, recordedVideoSize script constant, body/.viewer/.video-fit rules, and scrub-bar/header sizing; updated content/docs/concepts/interactive-viewer.mdx to describe the desktop window-fit behavior alongside the mobile stacking.

Risk Assessment

✅ Low: The change is bounded to one generated artifact, resolves both prior warnings with targeted test coverage, and satisfies every stated acceptance criterion; the only residual is a few pixels of scrub-bar clipping in a narrow race that the existing loadedmetadata re-fit usually repairs.

Testing

Ran the two targeted vitest files for the changed module and its consumer (viewer.test.ts, stop.test.ts — all green), then produced the real evidence in Chrome: generated viewer.html from both the base and target commits against a labelled 1280x720 recording and rendered each at Doggo's 1280x577 viewport. The baseline reproduces the reported bug (recording clipped 21px below the fold, scrub bar entirely off-screen, page not scrollable); the target commit fits the whole recording, native controls, and scrub bar on screen with the recorded aspect ratio preserved and overlays and scrub bar pixel-aligned to the fitted video. Also verified the legacy no-viewport-metadata path, the mobile stacked layout, a live resize sweep down to 450px height, the long-description header cap with Show more, and that the new body { overflow: hidden } does not strand side-panel content. Screenshots for before, after, legacy, mobile, long-description, and overlay-during-playback are in the evidence directory. One caveat: the mobile capture is 500x667 rather than 375 wide because macOS Chrome will not size a window narrower than 500px, but the max-width: 768px branch is the one exercised.

  • Evidence: BEFORE (base 1a8b119) @ 1280x577 — recording clipped, controls cut, scrub bar off-screen (local file: /var/folders/xh/qnn4w61965q9ml19t2yy0fk00000gp/T/no-mistakes-evidence/01KZY1WK38CDSXNKC43AADWN71/01-before-1280x577-clipped.png)
  • Evidence: AFTER (target eb28a83) @ 1280x577 — full recording, native controls, and scrub bar all visible (local file: /var/folders/xh/qnn4w61965q9ml19t2yy0fk00000gp/T/no-mistakes-evidence/01KZY1WK38CDSXNKC43AADWN71/02-after-1280x577-fits.png)
  • Evidence: AFTER, legacy session with no viewport metadata @ 1280x577 — fits once loadedmetadata supplies the aspect (local file: /var/folders/xh/qnn4w61965q9ml19t2yy0fk00000gp/T/no-mistakes-evidence/01KZY1WK38CDSXNKC43AADWN71/03-after-legacy-no-viewport-metadata-1280x577.png)
  • Evidence: AFTER, mobile branch @ 500x667 full page — stacked layout preserved and scrollable (local file: /var/folders/xh/qnn4w61965q9ml19t2yy0fk00000gp/T/no-mistakes-evidence/01KZY1WK38CDSXNKC43AADWN71/04-after-mobile-500x667-fullpage.png)
  • Evidence: AFTER, long description expanded @ 1280x577 — header capped at 50%, video re-fits below (local file: /var/folders/xh/qnn4w61965q9ml19t2yy0fk00000gp/T/no-mistakes-evidence/01KZY1WK38CDSXNKC43AADWN71/05-after-long-description-expanded-1280x577.png)
  • Evidence: AFTER, overlay during playback @ 1280x577 — action toast and ripple aligned inside the fitted video (local file: /var/folders/xh/qnn4w61965q9ml19t2yy0fk00000gp/T/no-mistakes-evidence/01KZY1WK38CDSXNKC43AADWN71/06-after-overlay-aligned-during-playback-1280x577.png)
Evidence: Measured geometry, before vs after, 1280x577 viewport
BEFORE (1a8b119):
video {top:170, bottom:598, w:762, h:428} viewport height 577 -> clipped by 21px
scrub {top:598, bottom:619} -> entirely below the fold
pageScrollable: false -> scrub bar unreachable

AFTER (eb28a83):
video {top:170, bottom:539, w:657, h:370} videoFullyVisible: true
scrub {top:539, bottom:560} scrubFullyVisible: true
rendered aspect 1.7757 vs recorded 1.7778 no distortion
overlayAlignedToVideo: true scrubAlignedToVideo: true

AFTER, legacy (no viewport metadata):
containerHasFitClassNow: true containerAspectRatioNow: "1280 / 720"
video {top:170, bottom:539, w:657, h:370} videoFullyVisible: true

AFTER, live resize sweep (no reload):
1280x900 -> 762x428 aspect 1.778 everythingVisible: true
1280x577 -> 657x370 aspect 1.778 everythingVisible: true
1280x450 -> 432x243 aspect 1.778 everythingVisible: true

AFTER, long description + Show more:
collapsed header 188px -> video 597x336
expanded header 289px (cap = 50% of 577) -> video 419x236, everythingVisible: true

AFTER, overlay ripple for recorded bbox centre (400,180) in a 1280x720 frame:
expected (273.6, 262.1) actual (273.6, 262.2) offset (0.0, 0.1) px
Evidence: Evidence generator used to produce the before/after viewer.html pair
import * as fs from 'fs';
import { generateViewer as generateAfter } from '@after/artifacts/viewer.js';
import { generateViewer as generateBefore } from '@before/artifacts/viewer.js';

const EV = process.env.EV!;

const entries = [
  { action: 'navigate http://localhost:3000', relativeTimeSec: 0.4, timestamp: '2026-08-13T10:00:00Z' },
  { action: 'screenshot dashboard', relativeTimeSec: 1.6, timestamp: '2026-08-13T10:00:02Z' },
  { action: 'click "New session"', relativeTimeSec: 2.9, timestamp: '2026-08-13T10:00:03Z', outcome: 'passed' as const },
  { action: 'type "proofshot viewer"', relativeTimeSec: 4.1, timestamp: '2026-08-13T10:00:04Z' },
  { action: 'screenshot result', relativeTimeSec: 5.4, timestamp: '2026-08-13T10:00:05Z' },
];

const base = {
  description:
    'Verify the ProofShot viewer keeps the whole recording, its native controls, and the scrub bar visible on a constrained 1280x577 browser viewport.',
  serverCommand: 'npm run dev',
  durationSec: 6,
  videoFilename: 'session.webm',
  entries,
  consoleErrorCount: 0,
  serverErrorCount: 0,
  consoleOutput: '[log] app ready\n[log] session started',
  serverLog: 'ready - started server on 0.0.0.0:3000',
};

// Modern session: recorded viewport is known at generation time.
fs.writeFileSync(`${EV}/after/viewer.html`, generateAfter({ ...base, viewport: { width: 1280, height: 720 } }));
fs.writeFileSync(`${EV}/before/viewer.html`, generateBefore({ ...base, viewport: { width: 1280, height: 720 } }));
// Legacy session: no recorded viewport, aspect ratio only knowable after loadedmetadata.
fs.writeFileSync(`${EV}/after-legacy/viewer.html`, generateAfter({ ...base, viewport: undefined }));

console.log('generated viewer.html for after / before / after-legacy');

// Long description: exercises the `.header { max-height: 50% }` cap and the Show more refit.
const long = Array.from({ length: 14 }, (_, i) =>
  `Paragraph ${i + 1}: a long verification note describing every interaction the agent performed during this session so the description overflows the header on a short viewport.`,
).join(' ');
fs.writeFileSync(
  `${EV}/after-long-desc/viewer.html`,
  generateAfter({ ...base, description: long, viewport: { width: 1280, height: 720 } }),
);

// Overlay alignment: a click whose element bbox sits at a known point in the 1280x720 recording.
fs.writeFileSync(
  `${EV}/after-overlay/viewer.html`,
  generateAfter({
    ...base,
    viewport: { width: 1280, height: 720 },
    entries: [
      {
        action: 'click "New session"',
        relativeTimeSec: 2,
        timestamp: '2026-08-13T10:00:02Z',
        outcome: 'passed' as const,
        element: {
          label: 'New session',
          bbox: { x: 300, y: 150, width: 200, height: 60 },
          viewport: { width: 1280, height: 720 },
        },
      },
    ],
  }),
);

Pipeline

Updates from git push no-mistakes

Step Agent (via) Nested agents
Review r1 claude (harness_cli) none
Review r2 claude (harness_cli) none
Review r2 claude (harness_cli) none
Build r1 claude (harness_cli) none
Test r1 claude (harness_cli) none
Document r1 claude (harness_cli) none
✅ **Intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

⚠️ **Review** - 1 warning
  • ⚠️ src/artifacts/viewer.ts:717 - The video is now height-driven and centered, but the scrub bar underneath it is still full-width, so the two no longer form one card. .video-wrapper gained align-items: center (line 717) while .video-container.video-fit is width: auto; max-width: 100% (line 725-730) and .scrub-bar keeps width: 100% (line 760). Concretely at 1280x577 with a 1280x800 recording: the panel content box is ~761px wide, available height ~371px, so the container resolves to 371 x ~594px and is centered — while the scrub bar spans the full 761px, overhanging ~83px on each side and breaking the joined border-radius: 8px 8px 0 0 / 0 0 8px 8px seam. Functionally the track still seeks correctly (it maps against its own rect), but the markers no longer sit under the video they annotate. Fix by making the scrub bar share the video's resolved width (e.g. move the aspect-fit sizing onto a shared inner element that wraps both, or set .scrub-bar { width: auto; align-self: stretch } inside an inner box sized by the video).
  • ⚠️ src/artifacts/viewer.ts:483 - body now has height: 100vh + overflow: hidden (line 480-483) and .header is flex: 0 0 auto (line 487), so the page can never scroll and the header can never shrink. The description renders 2-line-clamped with a "Show more" button (line 287, toggleDescription), which expands it in place with no cap. On a 577px-tall viewport a ~20-line description grows the header past ~550px; .viewer is flex: 1 1 auto; min-height: 0 (line 697-698) so it collapses toward zero, hiding the video, native controls, and scrub bar entirely — and with overflow: hidden there is no scrollbar to recover them. Before this change body was min-height: 100vh and the page scrolled. Suggested fix: cap header growth and let it scroll internally (.header { flex: 0 1 auto; max-height: 50%; overflow-y: auto }) so the viewer keeps a usable floor.
  • ℹ️ src/artifacts/viewer.test.ts:188 - The new regression assertions match whole CSS rule blocks verbatim, including declarations unrelated to the layout invariant being protected — the body assertion (line 188-195) pins the font-family list and background/color values, so any unrelated theme tweak fails a test named for video sizing. Narrow the assertions to the declarations that carry the invariant (height: 100vh, overflow: hidden, flex: 1 1 auto, min-height: 0).

🔧 Fix: Align scrub bar to fitted video and cap header height
1 warning still open:

  • ⚠️ src/artifacts/viewer.ts:1365 - The height-fit invariant is no longer enforced by CSS (.video-container.video-fit is now flex: 0 0 auto; width: 100%, so the card height is derived from the px width JS writes onto .video-wrapper). It is enforced only by fitVideoToAvailableSpace() at three hand-enumerated triggers: script parse (line 1365), resize (line 1366), toggleDescription (line 1294), and loadedmetadata (line 1745). One layout mutation is missed: initDescription() (line 1274-1286) reveals the "Show more" button inside a requestAnimationFrame, which fires after the line-1365 fit. For any description longer than the 2-line clamp, the header grows ~18px, .viewer shrinks by the same amount, and the card keeps its already-computed height — so it overflows the wrapper. .video-panel's 16px padding absorbs most of it, leaving ~2px of the 21px scrub bar clipped, and the loadedmetadata re-fit usually repairs it if metadata resolves after the rAF. Small in magnitude, but it is the same stale-measurement class the change exists to close. Earliest shared boundary: replace the ad-hoc call sites with a single ResizeObserver on .video-panel (safe — writing the wrapper width cannot resize a flex: 0 0 62%; overflow: hidden panel, so no observer loop), rather than adding a fourth trigger inside the rAF.
✅ **Build** - passed

✅ No issues found.

✅ **Test** - passed

✅ No issues found.

  • npx vitest run src/artifacts/viewer.test.ts — 5 passed
  • npx vitest run src/commands/stop.test.ts — 13 passed (viewer.html consumer)
  • Generated viewer.html from both 1a8b119 (before) and eb28a83 (after) via esbuild-bundled generateViewer, paired with a real 1280x720 VP9 recording whose top/bottom edges are labelled so clipping is visible
  • Chrome DevTools: loaded before/viewer.html at 1280x577 and measured video + scrub rects — video bottom 598px vs 577px viewport, scrub bar 598-619px off-screen, document.scrollHeight === innerHeight (unreachable)
  • Chrome DevTools: loaded after/viewer.html at 1280x577 — video 657x370 fully visible, scrub 539-560px visible, overlayAlignedToVideo and scrubAlignedToVideo true, rendered aspect 1.7757 vs recorded 1.7778
  • Legacy session (viewer generated with viewport: undefined, recordedVideoSize = null) at 1280x577 — video-fit class and aspect-ratio: 1280 / 720 applied after loadedmetadata, same fitted geometry
  • Mobile branch at 500x667 (Chrome minimum window width; matchMedia('(max-width: 768px)') confirmed active) — stacked layout, wrapper inline width cleared, page scrolls, full-page screenshot captured
  • Live resize sweep without reload at heights 900 / 577 / 450 — video re-fits each time (762x428 / 657x370 / 432x243), aspect 1.778 throughout, scrub always visible and width-matched
  • Long-description variant at 1280x577: clicked Show more — header capped at exactly 289px (50% of 577) with own scroll, video re-fit 597x336 -> 419x236, nothing clipped
  • Overlay accuracy: seeked to a click entry with element bbox (300,150,200x60) in a 1280x720 frame, polled for .ripple during playback — rendered centre within 0.1px of the proportional expected point inside the fitted video box
  • Regression probe for body { overflow: hidden }: switched to the Console tab at 1280x577 — timeline panel stays within the viewport with its own overflow-y: auto, video still fully visible
⚠️ **Document** - 1 info
  • ℹ️ docs/architecture.md:106 - docs/architecture.md:106-131 keeps a second full copy of the viewer layout diagram and feature inventory that content/docs/concepts/interactive-viewer.mdx owns. This change did not make the architecture.md copy stale (it states nothing about video sizing or page scrolling), so I updated only the owner and left the duplicate alone — but the two copies will keep drifting on every viewer change. Proposed follow-up: reduce the architecture.md section to a pointer at the concepts doc. Out of scope here, and a prior phase already reverted an architecture.md refresh for scope (325342c).
⚠️ **Lint** - 1 warning
  • ⚠️ src/environment/tmux.ts:103 - npx tsc --noEmit reports 3 pre-existing TS2339 "does not exist on type 'never'" errors in src/environment/tmux.ts:103 and src/environment/runtime.test.ts:223,226. They are untouched by this change (which only edits src/artifacts/viewer.ts and viewer.test.ts, both type-clean), and a prior phase's fix plus CI typecheck step was deliberately reverted for scope (334121f, then 4c70904 "Revert unrelated typecheck expansion"). There is no typecheck script or CI typecheck today, so nothing enforces this. Left unfixed: fixing requires editing unrelated functional and test files.
✅ **Push** - passed

✅ No issues found.

@alvarohulse
alvarohulse merged commit 5d687b3 into main Aug 13, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant