fix(cli): make native modules (sharp, onnxruntime) optional + soften inspect overlap#1476
Merged
Merged
Conversation
cdc70c1 to
aa9fb31
Compare
…inspect overlap
Aimed at `npx hyperframes` users (standalone and inside monorepos), where the
native modules `sharp` and `onnxruntime-node` can't install or load.
## Native modules are now optional, and never abort the CLI
`sharp` and `onnxruntime-node` are native modules: their platform binaries ship
as optional sub-dependencies that can fail to land on end-user installs
(--omit=optional, musl/glibc, monorepo hoisting, cross-platform lockfiles,
broken npx cache). Both powered only optional commands, yet both were wired as
hard `dependencies`, so on any platform where a binary can't install the whole
CLI failed to install. Moved both to `optionalDependencies` (alongside
@google/genai) so the core CLI always installs; the native-accelerated paths
light up only when present.
Runtime handling so a missing/unloadable binary degrades instead of crashing:
- `capture` (`contentExtractor.ts`): sharp was a static top-level
`import sharp from "sharp"`, so a load failure threw on module import —
before the inner try/catch — aborting the whole command. Now a guarded lazy
`await import("sharp")` that skips SVG captioning with an actionable warning.
Marked `external` in tsup so esbuild never bundles the native module.
- `remove-background` (`inference.ts`): both `onnxruntime-node` and `sharp`
are loaded here and genuinely required. The dynamic imports are now guarded
to throw an actionable "install / reinstall with optional deps" error
(surfaced cleanly by the command's existing try/catch) instead of a raw
"Cannot find module". New tests assert createSession rejects with that
guidance — before touching the model download — when either module is
unavailable.
`contactSheet.ts` also uses sharp but is already behind a dynamic-import
boundary wrapped in try/catch, so it was never a hard-fatal path.
## inspect: content-overlap as a warning, not a blocking error
The `content_overlap` layout-audit check shipped as `severity: "error"`, and
the audit exits non-zero when `errorCount > 0`, so `inspect` failed for
compositions that intentionally layer text. Downgraded to `severity: "warning"`
so it still reports (and prints the `data-layout-allow-overlap` opt-out hint)
without breaking exit codes. Reversible.
aa9fb31 to
c31a7b0
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Make
npx hyperframesrobust for end users — standalone and inside monorepos — where the native modulessharpandonnxruntime-nodecan't install or load. Plus a smallinspectchange.Root fix: native modules are optional, and never abort the CLI
sharpandonnxruntime-nodeare native modules. Their platform binaries ship as optional sub-dependencies that can fail to land on end-user installs —--omit=optional, musl/glibc, monorepo hoisting, cross-platform lockfiles, a broken npx cache. Both powered only optional commands, but both were wired as harddependencies, so on any platform where a binary can't install, the whole CLI failed to install. Moved both tooptionalDependencies(same bucket as@google/genai) so the core CLI always installs; native-accelerated paths light up only when present.Runtime handling so a missing/unloadable binary degrades instead of crashing:
capture(contentExtractor.ts): sharp was imported statically (import sharp from "sharp"at module top), so a load failure threw on module import — before the innertry/catch— aborting the entire command. Now a guarded lazyawait import("sharp")that degrades to skipping SVG captioning with an actionable warning. Also markedexternalintsupso esbuild never bundles the native module.remove-background(inference.ts): bothonnxruntime-nodeandsharpload here and are genuinely required. The dynamic imports are now guarded to throw an actionable "install / reinstall with optional deps" error (surfaced cleanly by the command's existingtry/catch) instead of a raw "Cannot find module". New unit tests assertcreateSessionrejects with that guidance — before touching the model download — when either module is unavailable.contactSheet.tsalso uses sharp but is already behind a dynamic-import boundary wrapped intry { } catch { /* non-critical */ }, so it was never a hard-fatal path.Net effect for
npx hyperframesusers: the CLI always installs;capturenever aborts on sharp;remove-backgroundfails with clear guidance when its native deps are missing; native-accelerated features work whenever the binaries are available.inspect: content-overlap as a warning, not a blocking errorThe
content_overlaplayout-audit check shipped asseverity: "error", and the audit exits non-zero whenerrorCount > 0, soinspectfailed for compositions that intentionally layer text. Downgraded toseverity: "warning"so it still reports (and prints thedata-layout-allow-overlapopt-out hint) without breaking exit codes. Reversible.Verification
tsc --noEmitcleanbuildsucceeds;sharpandonnxruntime-nodestill resolve in-repo (optional deps install by default)vitest— 116 tests pass acrossbackground-removal,capture,commands/capture,layout-audit.browser, including 2 new tests for the missing-native-module error paths