Skip to content

fix(cli): read actual composition dimensions in info and fix semver update check#194

Merged
vanceingalls merged 4 commits into
mainfrom
fix/cli-info-resolution-update-check
Apr 2, 2026
Merged

fix(cli): read actual composition dimensions in info and fix semver update check#194
vanceingalls merged 4 commits into
mainfrom
fix/cli-info-resolution-update-check

Conversation

@miguel-heygen

@miguel-heygen miguel-heygen commented Apr 2, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • info command now reads actual data-width/data-height from the root composition element instead of hardcoding 1920x1080 or 1080x1920 based on a parser heuristic that defaults to "portrait"
  • Update check now uses proper semver comparison instead of string inequality (!== VERSION). Previously reported "update available" when installed 0.2.1 and npm had 0.2.0

Part 2 of 5 in a stacked PR series fixing E2E test findings.

Test plan

  • npx hyperframes info on a 1920x1080 project shows "1920x1080" (not "1080x1920")
  • npx hyperframes info --json returns correct width/height fields
  • Version check no longer reports downgrade as available update

Comment thread packages/cli/src/utils/updateCheck.ts

@vanceingalls vanceingalls 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.

Review feedback

Important: Non-null assertions
packages/cli/src/commands/info.ts uses widthMatch[1]! and heightMatch[1]!. Same project convention issue as #193 — use widthMatch?.[1] ?? "" or a guard.

Note: No tests
Neither the semver comparison nor the dimension extraction have tests. Even one test for isNewerSemver("0.2.1", "0.2.0") === true would add confidence. The semver fix itself is clearly correct though.

Note: Regex brittleness
The dual-regex for data-width extraction works for all realistic compositions but is fragile. Ideally dimensions should come from the parser output rather than a separate regex pass.

@miguel-heygen miguel-heygen force-pushed the fix/cli-info-resolution-update-check branch from 2a564cf to 096f09b Compare April 2, 2026 17:03
@miguel-heygen

Copy link
Copy Markdown
Collaborator Author

Addressed:

  1. Non-null assertions: Replaced widthMatch[1]! and heightMatch[1]! with widthMatch?.[1] optional chaining.

  2. No tests: Fair point. The semver comparison is now delegated to compare-versions (a well-tested library), so we get their test coverage. For the regex dimension extraction, I agree a test would help but kept the scope focused on the fix for now.

  3. Regex brittleness: Agreed long-term. The parser should expose actual data-width/data-height from the root composition element. Filed as a follow-up.

@vanceingalls vanceingalls changed the base branch from feat/lint-determinism-clip-overlap-raf to graphite-base/194 April 2, 2026 21:09
miguel-heygen and others added 3 commits April 2, 2026 21:10
…pdate check

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace hand-rolled isNewerSemver with compare-versions (3KB, zero deps)
which handles prerelease tags (alpha, beta, rc) per the semver spec.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@vanceingalls vanceingalls force-pushed the fix/cli-info-resolution-update-check branch from 096f09b to 806f008 Compare April 2, 2026 21:10
@graphite-app graphite-app Bot changed the base branch from graphite-base/194 to main April 2, 2026 21:10
Address review: widthMatch[1]! and heightMatch[1]! replaced with
widthMatch?.[1] per project convention against ! assertions.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@vanceingalls vanceingalls force-pushed the fix/cli-info-resolution-update-check branch from 806f008 to 0694b9e Compare April 2, 2026 21:10
@vanceingalls vanceingalls merged commit 7f4e433 into main Apr 2, 2026
15 checks passed

Copy link
Copy Markdown
Collaborator

Merge activity

miguel-heygen added a commit that referenced this pull request Apr 3, 2026
…pdate check (#194)

## Summary

- **`info`** **command** now reads actual `data-width`/`data-height` from the root composition element instead of hardcoding 1920x1080 or 1080x1920 based on a parser heuristic that defaults to "portrait"
- **Update check** now uses proper semver comparison instead of string inequality (`!== VERSION`). Previously reported "update available" when installed `0.2.1` and npm had `0.2.0`

**Part 2 of 5** in a stacked PR series fixing E2E test findings.

## Test plan

- [x] `npx hyperframes info` on a 1920x1080 project shows "1920x1080" (not "1080x1920")
- [x] `npx hyperframes info --json` returns correct width/height fields
- [x] Version check no longer reports downgrade as available update
@miguel-heygen miguel-heygen deleted the fix/cli-info-resolution-update-check branch April 6, 2026 23:24
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.

3 participants