Repository navigation
fix(render): align container Chrome with the CLI - #5338
user-github-me wants to merge 4 commits into
Conversation
terencecho
left a comment
There was a problem hiding this comment.
Reviewed HEAD 6851c0447b4bb5f370bc7290bd2dfd1933b52201 against effective merge-base 3aa68869f7d4cec8b37cdfcb9cd539389b63abed. No introduced, reachable blocking defect found in this source-only review.
Audited: All nine changed paths, including existing BASE versions first; both container pins against the CLI managed build and actual executable resolution; preserved production-argument BeginFrame build probe and runtime screenshot fallback; fixture metadata, nested composition, discovery, shard scheduling and golden-file consumers. Also checked the unchanged chat composition behind its refreshed video pointer, relevant surviving regression assertions, and interactions between the pin, fixture, schedule and baseline changes. The prior #2817 launch/probe correction remains intact; #5207 addresses the separate CLI contrast-audit behavior.
Trusting / not independently validated: Chrome 152 native behavior/availability, actual LFS compiled/video contents, pixel/audio/PSNR outcomes and the author's runtime/corpus reports. Output blobs were inspected only as Git LFS pointer text. No tests, renderer, Chrome, container builds, CI or live services were run. Context coverage was targeted, not whole-repository semantic coverage.
Verdict: APPROVE
Reasoning: Both container pins match the CLI's main managed build while retaining exact-executable BeginFrame validation and fallback. The new regression case is wired into the existing harness and regular shard schedule without a source-level blocker.
— tai
|
Thanks for this, @user-github-me. This PR now has merge conflicts with |
Preview approval requiredThis pull request comes from a fork, so Mintlify won't build a preview until someone with write access approves commit |
miguel-heygen
left a comment
There was a problem hiding this comment.
Thanks for the rebase. The new pin matches the CLI's managed browser on current main (CHROME_VERSION = "152.0.7977.30" in packages/engine/src/services/chromeHostCeiling.ts). CI at 3e9883f7 is green, including the Cloud Run BeginFrame image contract built with Chrome 152 and all ten regression shards. So the bump doesn't disturb the other baselines, and the new gradient-text-layered-child fixture runs in the regular schedule.
Two places still ship Chrome 148, for you or a maintainer to decide whether they belong here or in a follow-up:
examples/k8s-jobs/Dockerfile.examplepinsCHROME_HEADLESS_SHELL_VERSION=148.0.7778.167. It's a one-line bump, and people copy that example as is.- AWS Lambda uses
@sparticuz/chromium148.0.0(packages/aws-lambda/package.json, with its arm64 pack hash inscripts/stage-arm64-image.ts). The gradient-text composition from #5117 will most likely still lose its glyphs there. That needs a matching Chromium pack and a new pin, so a separate PR makes more sense.
Reviewed head 3e9883f7. It merges cleanly with main. I didn't render locally and relied on the regression lanes.
Verdict: APPROVE
Reasoning: The pins match the CLI's browser, the GCP image contract and every regression shard pass at this head, and the remaining Chrome 148 sites are follow-ups.
Cloud Run and regression-test images use
chrome-headless-shell@152.0.7977.30, matching the CLI's managed browser. This restores child glyphs in gradient text when a transform creates a separate paint layer. A one-second nested-composition fixture covers the missing text and runs in the regular regression shard schedule.Fixes #5117. #5207 covers the separate contrast checker; the Cloud Run BeginFrame build contract from #2817 remains enforced. The deployment guide explains that existing services need an updated image to receive the browser fix.
The chat baseline preserves both changes now present across the branches: its Chrome 152 video stream and main's corrected AAC stream from #5956. They are combined with stream copy, without re-encoding. All chat source files, metadata and compiled HTML are identical between the two branches. Both streams retain their original compressed payloads, decoded samples, packet timing, duration, sizes, flags and side data; both inputs span 15 seconds with 450 video frames at 30 fps.
Validation on the updated branch:
The original Chrome change was validated on Linux/amd64 with the exact 148 and 152 executables: the new fixture failed all 100 visual checkpoints on 148 and passed all 100 on 152, and the then-current 50 regular in-process plus two distributed fixtures passed with 152. That earlier corpus run is not a fresh run of this updated branch. A fresh Docker rebuild was attempted here but external package/download failures prevented a current pinned-runtime render; the updated GitHub regression lane must verify compatibility before merge.