Skip to content

fix(cli): measure ink, one output-folder flag spelling, logo wordmark variant - #6891

Merged
miguel-heygen merged 7 commits into
mainfrom
fix/check-first-frame-measure-flags
Oct 31, 2026
Merged

miguel-heygen merged 7 commits into
mainfrom
fix/check-first-frame-measure-flags

Conversation

@miguel-heygen

@miguel-heygen miguel-heygen commented Oct 31, 2026 •

Copy link
Copy Markdown
Collaborator

What

Three small CLI gaps found while rebuilding a 77 s animated film with fast camera moves and light flashes from a reference video:

  • hyperframes measure ink <video> --at <s> --roi x,y,w,h reports the bounds, size and text lines of the ink in a region of one frame, in the video's own pixels. No numpy script needed to size text.
  • measure and snapshot both accept --out, --output and --output-dir (and snapshot -o) for the output folder. Before, measure frames --out and snapshot --output each rejected the other spelling.
  • media-use resolve --type logo --variant wordmark returns the brand's wordmark from theSVG (also mono, light, dark, ...). A brand without that variant is a miss, never the mark or an avatar in its place.

Why

Sizing text from its ink was the one step in the reverse workflow that needed hand-written code. The two flag spellings were an easy trap, and resolve --type logo only ever returned the default mark although theSVG ships a wordmark for many brands.

Related work

Partial: first_frame_empty false positive is not included, see below.

How

  • measure/ink.ts reuses the existing ink definition in measure/drawn.ts (inkOf: a pixel is ink when its colour is far from its neighbourhood's median), then takes bounds and row bands.
  • outArg and extract's out gain the aliases output and output-dir; snapshot's output gains out and output-dir (same pattern as tts.ts).
  • --variant goes through ctx to thesvgSearch. A variant bypasses the intent and entity caches on the way in, and is never saved back to them (no global-cache put, no entity, its recorded prompt carries the variant), so a later default request for the same intent or entity cannot return the wordmark. The avatar and favicon tiers return nothing for a variant.

first_frame_empty did not reproduce as reported. On main, a full-frame scene at z-index: 10 over a title at 0 s gives no first_frame_empty and no canvas_covered when the scene holds a div, an svg or text; a scene starting at 3 s gives none at 0 s. It fires only for a bare opaque slab with nothing in it, where frame 0 really is empty and canvas_covered also errors. Left unchanged.

Not done, needs a product call: text_below_floor and contrast_aa_failure in a reverse, where the original's own tiny text and colours trip them. Proposal: a --match-original mode that skips those two checks for text present in the reference.

Test plan

fails without the fix: measure/outFlags.test.ts (each command's output flag takes out, output, output-dir; red with the alias removed) and resolve.test.mjs "a logo variant is not reused for the default logo" (resolves the wordmark, then the default in the same project and in another project through the global cache; red with the variant saved back to the cache).

  • measure/ink.test.ts (bounds shifted to the region's place, one band per line, null when empty); breaking the +1 in the width makes it fail.

  • logo-provider.test.mjs (wordmark url, miss when absent, no avatar or favicon for a variant); ignoring the variant makes it fail. 23 pass, 3 runs.

  • Manual on a generated clip: measure ink --at 1 --roi 40,100,500,160 printed bounds [100,140,300,50] for a 300x50 white box; measure frames --out and --output both wrote their stills.

  • Unit tests added/updated

  • Manual testing performed

  • Documentation updated (if applicable)

  • Comments follow CONTRIBUTING.md "Comments": they say why, not what, and a bug fix says what the code must do and how to reproduce the bug

… logo --variant wordmark

measure ink reports the bounds, size and lines of text in a region of one frame, so a rebuild can be sized without a script. measure and snapshot take --out and --output for the same folder. media-use resolve --type logo --variant wordmark returns the brand's wordmark when theSVG has one.

@heygengenesis heygengenesis Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Requesting changes at 988a0b5d. Two blockers:

1. A --variant resolve leaves its artwork in the default logo's cache slot. --variant skips reuse on its own way in (forced), but run() still saves the fetched wordmark like any other result: appendRecord(projectDir, record) plus cachePut(fullPath, record), with provenance.prompt = intent and entity. None of the readers look at provenance.variant:

  • findByPrompt in lib/manifest.mjs
  • findByEntity
  • cacheGet / cacheGetByEntity in lib/cache.mjs

Repro: resolve --type logo --intent "Slack logo" --variant wordmark, then resolve --type logo --intent "Slack logo" in the same project, or in any project through the global cache. The second call returns the wordmark as cached (.find gives back the first record that matches). This is the reverse of the swap the PR guards against.

Fix: either keep variant records out of reuse (don't cachePut, and don't key them on the bare intent), or make the four lookups reject a record whose provenance.variant is set and not "default". Add a test that resolves a variant and then the default for the same intent and expects the default mark.

2. The body has no fails without the fix: line. This lane counts it as a fix PR (fix(cli)), and the --out/--output change fixes a bug: each command rejected the other spelling. The body needs a fails without the fix: line naming a test that fails without the fix. Right now there is no test for the aliases at all: no test parses measure frames --output, measure extract --output, or snapshot --out and checks they reach out/output. The test plan says the ink and variant tests fail when their code is broken, but neither appears on a fails without the fix: line.

Smaller, not blocking:

  • The body says the aliases are output (measure) and out (snapshot). The code also adds output-dir to all three. Update the body, or drop output-dir if it wasn't meant.
  • The GitHub title is feat(cli): …, but the lane pins it as fix(cli): … check false positives in a reverse. The first_frame_empty part was left out (explained in the body). Make the title match what ships.

Looks right otherwise: measureInk bounds and bands (with +1), projectOf moved to media.ts without changing what it does, and the avatar and favicon tiers returning nothing for a variant.

@miguel-heygen miguel-heygen changed the title feat(cli): measure ink reads text size, --out and --output both work, logo --variant wordmark fix(cli): measure ink, one output-folder flag spelling, logo wordmark variant Oct 31, 2026

@heygengenesis heygengenesis Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Approving 1fcd3a00. Both blockers from my last review are fixed.

Variant logo no longer reused for the default logo. A variant resolve now records its prompt as <intent> [<variant>], leaves out entity, and skips cachePut. None of the four lookups can return it for a default request any more. I checked it two ways:

  • The new resolve.test.mjs test, "a logo variant is not reused for the default logo, in the project or the global cache", passes: 63/63.
  • With resolve.mjs put back to 988a0b5d, the same test fails: 62 passed, 1 failed. So the fails without the fix: line is accurate.

fails without the fix: line. It is present and names measure/outFlags.test.ts and the resolve test. The body now also lists the output-dir alias, and the title matches what ships.

CI run 41246115131 is green.

Nit, not blocking: outFlags.test.ts checks the alias definitions rather than parsing argv through citty. That is fine because citty resolves aliases into the main key, but a parse-level case would also cover the required: true --out on extract.

@miguel-heygen
miguel-heygen added this pull request to the merge queue Oct 31, 2026
Merged via the queue into main with commit 8a0b8a9 Oct 31, 2026
116 of 119 checks passed
@miguel-heygen
miguel-heygen deleted the fix/check-first-frame-measure-flags branch October 31, 2026 22:43
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