Skip to content

fix(review): bound mobile visual-capture thumbnails and give up on a missing preview build - #7410

Merged
JSONbored merged 1 commit into
mainfrom
fix/visual-capture-mobile-thumb-and-preview-loading
Jul 20, 2026
Merged

fix(review): bound mobile visual-capture thumbnails and give up on a missing preview build#7410
JSONbored merged 1 commit into
mainfrom
fix/visual-capture-mobile-thumb-and-preview-loading

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

  • Mobile "before"/"after" screenshots in the "Visual preview" PR comment table rendered at full,
    native (unbounded height) size instead of a bounded thumbnail. capturePage's display-thumbnail
    generation was gated to viewportName === "desktop" only, on the assumption that a 390px-wide
    mobile capture was already close enough to the table's display width to skip downscaling — true
    for width, but shot.ts captures fullPage: true, so a mobile screenshot's HEIGHT is just as
    unbounded as desktop's. downscaleForDisplay itself also only bounded width (no fit/height), so
    removing the desktop-only gate alone would not have fixed a narrow-but-very-tall image. Fixed both:
    the downscale now bounds width AND height (fit: "inside", mirroring downscaleForVision's
    existing pattern), and thumbnail generation + the beforeThumbUrlMobile/afterThumbUrlMobile
    fields now apply to both viewports, threaded through to the comment table's mobile row.
  • The "after" cell's loading placeholder never resolved for a repo with no discoverable preview-build
    signal at all (e.g. a frontend with no Workers Builds CI configured). buildCapture only fed the
    existing poll-budget give-up logic (preview-poll-budget.ts) when getPreviewBuildState returned
    "building"/"succeeded"; the "absent" state (no matching check-run found) was a silent no-op,
    so previewPending/previewFailed both stayed false forever and the "Rendering preview…" spinner
    never re-evaluated to a terminal state on any later review pass. Fixed by feeding "absent" into
    the same poll-budget-then-give-up treatment as "building", skipped when actions_fallback is
    configured (that feature already owns the "found nothing" case and needs the previewPending gate
    free to dispatch).

Closes #7405

Scope

  • The PR title follows type(scope): short summary Conventional Commit format.
  • This PR is focused and does not mix unrelated backend, UI, MCP, docs, dependency, and deploy changes.
  • This follows CONTRIBUTING.md and does not reintroduce GitHub Pages, VitePress, site/, or CNAME.
  • I linked a currently open issue this PR resolves (Closes #7405).

Validation

  • git diff --check
  • npm run actionlint (via npm run test:ci)
  • npm run typecheck
  • npm run test:coverage locally, unsharded — 100% statements/lines/functions on both changed
    source files (src/review/visual/capture.ts, src/selfhost/stubs/image-downscale.ts), 100%
    lines on src/review/unified-comment-bridge.ts (the one pre-existing branch/line gap on each
    file — capture.ts:400, unified-comment-bridge.ts:81 — predates this change and is outside
    the diff).
  • npm run test:workers
  • npm run build:mcp
  • npm run test:mcp-pack
  • npm run ui:openapi:check
  • npm run ui:lint
  • npm run ui:typecheck
  • npm run ui:build
  • npm audit --audit-level=moderate — pre-existing adm-zip/github-actionlint advisory (no fix
    available upstream), unrelated to this change and present on main before this PR.
  • New/changed behavior has unit tests for every new branch: both-bound (width+height) downscale
    math, both-viewport thumbnail generation and its threading through the comment table, and the
    "absent" build-state's poll-then-give-up path with and without actions_fallback configured.

Safety

  • No secrets, wallet details, hotkeys, coldkeys, user PATs, private keys, raw trust scores,
    private rankings, or private maintainer evidence are exposed.
  • Public GitHub text stays sanitized, low-noise, and does not imply compensation guarantees or
    optimization tactics.
  • Auth, cookie, CORS, GitHub App, Cloudflare, or session changes include negative-path tests. — N/A,
    no auth/session/CORS surface touched.
  • API/OpenAPI/MCP behavior is updated and tested where needed. — no public API/OpenAPI/MCP surface
    changed; this only affects the shape of the bot-generated PR comment table.
  • UI changes use live API data or real empty/error/loading states, not production mock/demo fallbacks. — N/A, no apps/loopover-ui changes.
  • Visible UI changes include a UI Evidence section — N/A, see Notes below.
  • Public docs/changelogs are updated where needed — N/A, no doc-visible behavior contract changed.

UI Evidence

Not applicable — this is a backend-only fix to the bot's own visual-capture pipeline
(src/review/**, src/selfhost/**), not a change to apps/loopover-ui. The rendered artifact this
PR fixes is the bot-generated PR comment table on OTHER repos' pull requests, which can't be
meaningfully screenshotted from a local dev server the way an apps/loopover-ui page can — it's
already covered by the unit tests exercising the exact HTML the comment builder emits (see
test/unit/visual-collapsible.test.ts's new mobile-thumbnail assertions), and will be verified
against a real live PR after the next beta cut/deploy, per this repo's established
build→cut-beta→deploy→verify cycle.

Notes

  • Both fixes were found together while auditing a live, reported incident on a real repo review (huge
    unresized mobile screenshots, and an "after" cell stuck on the loading placeholder). They're bundled
    in one PR since they're the same investigation, touch overlapping code in capture.ts, and were
    reported together.

…missing preview build

Mobile screenshots rendered at full-page native size in PR comments because the
display-thumbnail downscale only bounded width, and thumbnail generation was
gated to desktop only. Also, when no preview-build check-run is ever found for a
repo (no Workers Builds CI), the "after" cell spun on the loading placeholder
forever instead of eventually giving up honestly, since that state was never fed
into the existing poll-budget mechanism.

Closes #7405
@JSONbored JSONbored self-assigned this Jul 20, 2026
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@JSONbored
JSONbored merged commit 416a752 into main Jul 20, 2026
12 of 13 checks passed
@JSONbored
JSONbored deleted the fix/visual-capture-mobile-thumb-and-preview-loading branch July 20, 2026 07:36
@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 20, 2026
@codecov

codecov Bot commented Jul 20, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.21%. Comparing base (cd52bed) to head (6d44548).
⚠️ Report is 5 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7410      +/-   ##
==========================================
- Coverage   91.21%   91.21%   -0.01%     
==========================================
  Files         716      716              
  Lines       72789    72797       +8     
  Branches    20859    20863       +4     
==========================================
+ Hits        66392    66399       +7     
+ Misses       5355     5354       -1     
- Partials     1042     1044       +2     
Flag Coverage Δ
shard-1 35.89% <0.00%> (-0.02%) ⬇️
shard-2 41.23% <18.18%> (-0.10%) ⬇️
shard-3 38.96% <45.45%> (+<0.01%) ⬆️
shard-4 39.93% <90.90%> (+<0.01%) ⬆️
shard-5 27.13% <9.09%> (-0.01%) ⬇️
shard-6 31.20% <0.00%> (-0.05%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/review/unified-comment-bridge.ts 99.62% <100.00%> (ø)
src/review/visual/capture.ts 95.86% <100.00%> (+0.12%) ⬆️

... and 2 files with indirect coverage changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(review): visual-capture table renders oversized mobile thumbnails and never gives up on a missing preview build

1 participant