Skip to content

fix(cli): label SVG contact sheet thumbnails by file name on Windows - #6732

Merged
jrusso1020 merged 1 commit into
heygen-com:mainfrom
LukeTheoJohnson:fix/svg-contact-sheet-labels-windows
Nov 9, 2026
Merged

jrusso1020 merged 1 commit into
heygen-com:mainfrom
LukeTheoJohnson:fix/svg-contact-sheet-labels-windows

Conversation

@LukeTheoJohnson

Copy link
Copy Markdown
Contributor

What

The SVG contact sheet from capture labels every thumbnail with the full path to the SVG on Windows, and the 60-character cell limit cuts it off before you get to the file name:

1. C:\Users\me\site\capture\assets\svgs\l...

With the names gone agents can't tell which thumbnail is which file.

Why

createSvgContactSheet builds each path with join(dir, f) and then gets the label with p.split("/").pop(). join uses \ on Windows, so there's no / to split on and you just get the whole path back. The scroll and snapshot sheets in the same file already use basename.

Related work

Refs #5301, the last change to this function (raster formats in contact sheets).

How

Swapped p.split("/").pop()! for basename(p), which the file already imports.

Test plan

  • Unit tests added/updated: labels each SVG thumbnail with its file name spies on composite on the same sharp instance the code loads and checks the label reads 1. logo-mark. Fails on main on Windows and passes with the change. Linux and macOS pass either way.
  • Manual testing performed: ran bun run --cwd packages/cli test src/capture/contactSheet.test.ts on Windows 11, 17 passed.
  • Documentation updated (if applicable): n/a
  • Comments follow CONTRIBUTING.md "Comments": no comments added

oxlint, oxfmt, check-comment-citations and comment-ratchet are clean. The packages/cli typecheck has the same 20 errors as main, none in these files.

@LukeTheoJohnson
LukeTheoJohnson marked this pull request as ready for review October 29, 2026 08:09
SanHsien added a commit to SanHsien/hyperframes that referenced this pull request Oct 30, 2026
…name collision, Windows contact-sheet labels)

publishProject seeds usedArchivePaths from fileContents.keys(), so external files no longer
overwrite existing _ext/ names; contactSheet uses node:path basename for Windows paths.
Both match the upstream PR diffs line for line; DECISIONS records the adoption.

Executed from the roadmap's near-term table; the finished row is removed in
the same commit.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: SanHsien <34234698+SanHsien@users.noreply.github.com>

@jrusso1020 jrusso1020 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed at a07b25ea9214f89ab2f6ce566d63257feaa5509d.

I checked the whole contact-sheet function. Node basename handles native Windows separators and leaves the existing thumbnail labels and pagination intact. No introduced blocker found.

I ran the exact-head contactSheet suite: 4 tests passed with borrowed dependencies. This was Linux, not a native Windows run. The commit is unsigned and needs author re-signing before queueing.

Need: yes · Size: ok · Reuse: existing implementation and tests · Simpler: already bounded · Structure: fine
Risk: low - bounded change; validation limits are stated above.

Verdict: APPROVE

Reasoning: I checked the whole contact-sheet function. Node basename handles native Windows separators and leaves the existing thumbnail labels and pagination intact. No introduced blocker found.

createSvgContactSheet built each path with path.join and then took the
label with split("/"), so on Windows the label was the whole path and
the 60-character cell limit cut the file name off. Use basename, as the
scroll and snapshot sheets in the same file already do.
@LukeTheoJohnson
LukeTheoJohnson force-pushed the fix/svg-contact-sheet-labels-windows branch from a07b25e to b10fd34 Compare November 7, 2026 22:50
@LukeTheoJohnson

Copy link
Copy Markdown
Contributor Author

Re-signed the commit, no content changes.

@jrusso1020
jrusso1020 added this pull request to the merge queue Nov 9, 2026
Merged via the queue into heygen-com:main with commit 0ca2b1a Nov 9, 2026
70 checks passed
@miguel-heygen miguel-heygen mentioned this pull request Nov 9, 2026
3 of 4 tasks
SanHsien added a commit to SanHsien/hyperframes that referenced this pull request Nov 9, 2026
…ve more; advance watermark

Twelve commits, PRs 7453-7468, issues 7457-7465 and eleven pin moves are
recorded with their evidence in the continued ledger, DECISIONS and UPSTREAM.
Adopted: heygen-com#7338, heygen-com#7459, heygen-com#7463, heygen-com#7454, heygen-com#7456 and the heygen-com#7439 docs; heygen-com#6732 was
already ported. Deferred: heygen-com#7455, heygen-com#7437. Not applicable: heygen-com#7453, heygen-com#7269, heygen-com#7464.
The watermark moves to 9e346f9, PR heygen-com#7468, issue heygen-com#7465; four new PRs are
pinned and the eleven moved pins re-pinned at their current heads. FORK.md
records the fork-only differences in the heygen-com#7338 port; the roadmap tracks the
standalone scripts that keep a bare mkdtemp and issue heygen-com#7458.

Co-Authored-By: Claude Sonnet <noreply@anthropic.com>
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.

2 participants