✨ Align core SDK option contracts#283
Merged
Merged
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This was referenced May 31, 2026
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Vizzly - Visual Test ResultsCLI Reporter - Waiting for buildNo builds received yet for this pull request.
|
Robdel12
added a commit
that referenced
this pull request
May 31, 2026
## Why This PR aligns the browser-driven SDKs: Storybook and static-site. These are not just wrappers around the CLI. They discover stories/pages, launch browsers, capture screenshots, create cloud builds, attach metadata, send screenshots, and finalize the build. That makes them easy places for option drift to hide. A user can set comparison, browser, request, build, or metadata options and reasonably expect the same contract as the core CLI. Before this pass, those options were too blended together. Some values affected Playwright capture, some affected Vizzly requests, some became screenshot metadata, and some could be dropped before build creation. The goal is that these SDKs behave like first-class capture runners. Browser options stay with the browser. Vizzly routing/comparison options go to Vizzly. User metadata goes into the metadata bag. Cloud builds get finalized into an honest state. ## What changed ### Shared web SDK behavior - Builds cloud run options through small helpers that combine Vizzly config and git metadata into the same shape the core runner expects. - Preserves `threshold`, `minClusterSize`, `parallelId`, PR number, branch, commit, message, environment, eager mode, and build name when creating cloud builds. - Finalizes empty cloud runs instead of leaving builds open. - Finalizes failed capture runs as unsuccessful. - Reads plugin package versions from `package.json` so metadata cannot drift from the published package. - Uses SDK config schema helpers for concurrency defaults. ### Storybook - Validates positive integer concurrency. - Supports explicit browser/capture controls including `--no-headless`, `--timeout`, and `--request-timeout`. - Separates Playwright screenshot timeout from Vizzly request timeout. - Produces stable screenshot metadata for story ID, story title, story name, viewport name, viewport dimensions, browser, URL, threshold, min cluster size, full-page mode, and user properties. - Documents the sanitized `Component-Story@viewport` naming format the code actually uses. - Moves stale utility coverage into the active Node test suites. ### Static-site - Validates positive integer concurrency. - Supports explicit browser/capture controls including `--no-headless`, `--request-timeout`, and `--no-use-sitemap`. - Preserves per-page overrides when merging page config. - Matches page config paths with or without a leading slash. - Produces stable screenshot metadata for browser, viewport, viewport dimensions, URL, full-page mode, and user properties. - Passes Vizzly request timeout to the Vizzly client without mixing it into Playwright capture timeout. - Updates sample pages/config/docs to show the supported shape. ## Properties and options This PR sits on top of the core contract from #283: `properties` is user metadata, while SDK/config options stay top-level. The web SDKs still add framework-generated metadata needed for stable baselines, but user-supplied `properties` are treated as the user’s bag and merged intentionally rather than acting as an untyped option tunnel. The reason that distinction matters is practical: `threshold`, `minClusterSize`, `requestTimeout`, `buildId`, and similar values change behavior. They should be parsed and validated as options, not accidentally stored as metadata. ## Testing - Cloud run option construction from Vizzly config and git metadata. - Finalizing empty cloud runs and failed capture runs. - Storybook/static-site screenshot metadata generation. - Request timeout versus screenshot timeout behavior. - CLI option parsing and validation. - Config merge behavior, page pattern matching, crawler/task behavior, package exports, plugin metadata, and SDK integration paths. These are mostly integration-style tests around the SDK boundary. A later E2E visual-diff matrix should still exercise a real fixture site and assert actual diff outcomes through the CLI/API/dashboard path. ## Verification - `pnpm --dir clients/storybook test`: 181 passed. - `pnpm --dir clients/static-site test`: 211 passed. - Covered by the full preservation branch verification before the stack was split. - Stack cleanup verification: `git diff --check` and PR file-list checks for #283 through #287.
Robdel12
added a commit
that referenced
this pull request
May 31, 2026
## Why This PR aligns the test-runner SDKs: Vitest and Ember. These integrations are close to the user’s actual test code, so the contract has to be exact. When someone calls `expect(...).toMatchScreenshot(...)` or `vizzlyScreenshot(...)`, they need to know which options affect browser capture, which options affect Vizzly comparison/routing, which options are metadata, and when a visual diff fails the test. Before this pass, that boundary was fuzzy. Vitest could pass Vizzly-only options toward browser screenshot APIs. Element captures could receive page-only `fullPage` behavior. Ember could not reliably bridge build ID, request timeout, viewport, threshold, and min-cluster settings from browser test code to the node screenshot server and then to Vizzly. The goal here is boring parity: Vitest, Ember, the JS client, and the CLI should describe the same screenshot in the same conceptual way. ## What changed ### Vitest - Discovers `failOnDiff` from `.vizzly/server.json` and exposes it to the browser context with server URL and build ID. - Removes unused plugin options from `vizzlyPlugin()` so screenshot behavior lives on matcher calls. - Supports both named and options-only matcher call shapes. - Strips Vizzly-only fields before calling Playwright/Vitest screenshot APIs: `properties`, `threshold`, `minClusterSize`, `failOnDiff`, `buildId`, and `requestTimeout`. - Prevents element screenshots from receiving page-only `fullPage`. - Adds stable screenshot metadata for framework, browser, URL, viewport, viewport dimensions, threshold, min cluster size, full-page mode, and user properties. - Treats new baselines as successful captures. - Fails assertions for real visual diffs only when `failOnDiff` resolves to true. - Updates matcher types for supported Playwright screenshot options plus Vizzly comparison/request options. ### Ember - Injects screenshot URL, build ID, and fail-on-diff settings into the test page before navigation. - Forwards `buildId` and `requestTimeout` from the screenshot server to Vizzly. - Sets viewport before capture when the helper asks for a specific size. - Keeps `fullPage` on page captures and strips it from selector captures. - Forwards `threshold`, `minClusterSize`, `buildId`, `requestTimeout`, viewport, and user properties in a stable payload. - Allows per-screenshot `failOnDiff` to override launcher/server settings. - Improves the no-server error so users are pointed to `vizzly tdd start` or `vizzly run`. - Keeps the Chromium CI stability args small and directly tested. ## Properties and options This PR follows the core contract from #283. `properties` is the user metadata bag. Options that change capture, comparison, routing, request timeout, build grouping, or failure behavior stay as explicit options. That is why the bridge code strips Vizzly-only fields before calling browser screenshot APIs and forwards them separately to Vizzly. It keeps browser capture behavior, comparison behavior, and metadata from quietly bleeding into one another. ## Testing - Vitest plugin discovery of local server config and `failOnDiff`. - Vitest matcher argument normalization, browser detection, metadata generation, element/page capture behavior, and option filtering. - Ember launcher injection of build/fail settings. - Ember screenshot-server request payloads, viewport handling, request timeout forwarding, selector/full-page behavior, and error handling. - Ember test-support payload construction, reserved metadata behavior, injected/per-call build IDs, request timeout, and per-screenshot diff behavior. This is good boundary coverage for option flow. It still does not replace a product-level E2E visual-diff fixture that proves the whole CLI/API/UI loop with a real changed screenshot. ## Verification - `pnpm --dir clients/vitest test`: 23 passed. - Ember package tests were verified in the preservation branch: 49 passed. - Covered by the full preservation branch verification before the stack was split. - Stack cleanup verification: `git diff --check` and PR file-list checks for #283 through #287.
Robdel12
added a commit
that referenced
this pull request
May 31, 2026
## Why This PR aligns the Ruby and Swift SDKs with the contract established by the core, web, Vitest, and Ember slices. These SDKs are especially important because users do not see the main JavaScript implementation details. They just call `Vizzly.screenshot(...)`, `client.screenshot(...)`, or `app.vizzlyScreenshot(...)` and expect the same build grouping, metadata, request timeout, comparison, and diff-failure behavior they get elsewhere. Before this pass, Ruby and Swift were close but not fully aligned. Build IDs and request timeouts could be treated like metadata. `failOnDiff` was not consistently available. Swift XCTest helpers duplicated metadata construction across overloads. Ruby accepted flexible option shapes, but the behavior was not crisp enough about what was a real SDK option versus user metadata. The goal is parity without overbuilding: one conceptual contract, expressed idiomatically in Ruby and Swift. ## What changed ### Ruby SDK - Adds `fail_on_diff:` and honors `VIZZLY_FAIL_ON_DIFF` plus discovered `.vizzly/server.json` settings. - Honors `VIZZLY_ENABLED=false` and reports disabled/ready state consistently. - Accepts snake_case options and JS-style camelCase aliases where useful: `min_cluster_size`/`minClusterSize`, `full_page`/`fullPage`, `build_id`/`buildId`, and `request_timeout`/`requestTimeout`. - Sends `buildId` as a request field, not screenshot metadata. - Applies `request_timeout` to the HTTP read timeout in milliseconds. - Treats `properties` as the user metadata bag. - Strips reserved/config options from `properties`, promotes them to the correct behavior when no top-level value exists, and emits a warning. - Raises on both current 200-style and older 422-style TDD diff responses when fail-on-diff is enabled. - Exposes both snake_case and camelCase info fields where that helps Ruby users and cross-SDK tooling. ### Swift SDK - Adds optional `failOnDiff` override and honors `VIZZLY_FAIL_ON_DIFF` plus discovered server config. - Adds optional `fullPage`, `buildId`, and `requestTimeout`. - Converts millisecond request timeout into `URLRequest.timeoutInterval`. - Sends `buildId` outside `properties`. - Preserves `fullPage: false` as an intentional value. - Includes a stable properties object in payloads. - Handles both current and legacy TDD diff response shapes. ### Swift XCTest helpers - Centralizes XCTest metadata construction in `VizzlyXCTestMetadata`. - Adds app/test/element metadata through one merge path. - Includes platform/device/OS/viewport metadata for app screenshots. - Includes platform metadata and element type for element screenshots. - Keeps user-provided properties as the intentional override layer. - Forwards `buildId` and `requestTimeout` through helper overloads. ### Docs and package surface - Ruby README examples now show the screenshot-level options where they are actually accepted. - Swift README and integration docs describe the current API signatures and current `vizzly run "..."` command style. - Swift package metadata now covers both the client and XCTest helper tests. ## Testing - Ruby tests cover disabled state, info shape, env/server-config fail-on-diff, current and legacy diff responses, fractional thresholds, nested viewport metadata, aliases, zero values, build ID routing, timeout conversion, and reserved `properties` warnings. - Swift tests cover info/fail state, payload shape, explicit false full-page behavior, build ID routing, nested viewport metadata, default comparison behavior, and XCTest metadata merging. The Ruby follow-up for the `properties` contract was re-run after the stack split. The broader real visual-diff E2E matrix is still intentionally left for a focused follow-up so this already-large SDK slice stays reviewable. ## Verification - Ruby preservation-branch verification: 20 tests / 55 assertions. - Ruby post-split verification after properties warning work: `bundle exec rake test` in `clients/ruby`: 21 runs, 64 assertions, 0 failures. - Swift preservation-branch verification: `swift test --filter VizzlyClientTests`: 17 passed. - Covered by the full preservation branch verification before the stack was split. - Stack cleanup verification: `git diff --check` and PR file-list checks for #283 through #287.
Robdel12
added a commit
that referenced
this pull request
May 31, 2026
## Why
This PR is the docs polish slice for the SDK alignment stack. The code
PRs make the option contract real; this one makes sure the public docs
describe the product users actually get.
The docs had fallen behind in a few important ways. They did not clearly
show the current cloud setup flow. JSON output examples were stale.
Browser flag docs did not include Ember. Some command examples still
used older names or older payload shapes. And, most importantly after
the latest cleanup, the screenshot options docs did not clearly separate
user metadata from SDK/config options.
That separation matters. `properties` is the user key/value metadata
bag. Options like `threshold`, `minClusterSize`, `fullPage`, `buildId`,
and `requestTimeout` are behavior/request options and should stay
top-level. If docs blur that line, users copy the wrong shape and the
SDK has to guess what they meant.
## What changed
### README
- Adds split agent-init flags so users can install only the agent skill
or skip it when they only want config.
- Clarifies the cloud path: `vizzly login`, `vizzly project link`, then
`vizzly run "..." --wait`.
- Updates the JS client screenshot example to show structured viewport
metadata, `fullPage`, and `requestTimeout`.
- Clarifies that `properties` is user metadata.
- Clarifies that SDK/config options stay top-level and that
transport/request fields like `requestTimeout` and `buildId` are not
stored as screenshot metadata.
- Expands upload examples with `--threshold`, `--min-cluster-size`,
`--batch-size`, `--upload-timeout`, `--upload-all`, `--parallel-id`, and
`--allow-no-token`.
### JSON output reference
- Reworks command examples to show the standard `{ status, data }`
wrapper.
- Updates `run`, `run --wait`, TDD commands, builds, comparisons,
config, baselines, API, upload, preview, status, init, and project
examples to match current payload shapes.
- Clarifies that field selection applies before the standard output
envelope is wrapped.
- Updates project command docs from stale `project:*` examples to
current `project link` and `projects` flows.
- Adds current preview fields such as new bytes, reused blob count,
dedupe ratio, and base path.
### Browser flags
- Documents that Ember uses the same sandbox and CI-stability subset as
the browser-based SDKs.
- Adds the Ember launcher path to the source list reviewers should check
when browser flags change.
- Clarifies why Ember uses a smaller subset because Testem owns the rest
of the browser lifecycle.
## Testing and confidence
This is docs-only, so the useful verification is consistency against the
code PRs and diff hygiene. The docs now say the same thing the core/Ruby
changes enforce: user metadata belongs in `properties`; options that
change behavior stay top-level.
The remaining testing gap is not in this docs PR but in the broader
product stack: we still want a focused follow-up with real E2E
visual-diff tests that exercise the CLI/API/dashboard path with an
intentionally changed fixture.
## Review guide
1. Start with `README.md` and check the screenshot/options examples
against #283 and #286.
2. Review `docs/json-output.md` against the JSON output work in #283.
3. Check `docs/browser-flags.md` against the Storybook/static-site/Ember
browser launchers from the SDK PRs.
## Verification
- Docs-only slice.
- `git diff --check` passed after stack cleanup.
- Final stacked branch differs from the original preservation branch
only by intentional docs/code split changes and the removed custom
plugin example.
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
1 participant
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.
Why
This PR is the foundation for the SDK alignment stack. The later SDK PRs depend on this one because this is where the CLI, JS client, uploader, local TDD server, API helpers, config loading, environment loading, and public types agree on the same option contract.
The problem was not one isolated bug. It was drift across entry points. Users could set an option in one place and have it work, then set the same conceptual option through config, env,
vizzly run,vizzly upload, TDD, or the JS client and have it get dropped, renamed, serialized into the wrong place, or returned in a different result shape.That is especially risky for visual testing because options are part of the product contract. If we say
threshold,minClusterSize,buildId,parallelId, PR metadata, or request timeout exists, users should be able to trust where it goes and what it changes.What changed
Option and config contract
thresholdandminClusterSize.VIZZLY_THRESHOLDandVIZZLY_MIN_CLUSTER_SIZE.build.*,comparison.*,parallelId) instead of stale top-level aliases.requestTimeoutandbuildIdas transport/request options rather than screenshot metadata.Screenshot properties contract
propertiesas the user metadata bag.propertieswhen users accidentally put them there.This matters because silently accepting
properties: { threshold: 0.1 }as metadata is misleading. The user meant comparison behavior, not a random stored metadata key. We now normalize it into the right place and tell them what happened.CLI behavior
--tokenbefore plugin loading so plugins see the same auth context as normal commands.upload --min-cluster-size.upload --waitfail when completed comparisons contain failures, including JSON mode.run --json --waitwait for the build result before emitting final JSON.api --dataunless the method is POST.preview --basepath resolution and JSON output.project linkrespect a CLI token override before stored login state.JS client and local TDD
getVizzlyInfo()stable values and reports missing optional values asnull.configure({ failOnDiff })update an existing configured client.VIZZLY_FAIL_ON_DIFFand local daemon server info through the same behavior path.buildIdas a request build ID.requestTimeoutto the HTTP request.failOnDiffis enabled.API, uploader, and build results
uploadAlltruly bypass SHA deduplication.flush()result when available.Public types
propertiesfor metadata.Testing
This PR adds focused coverage at the boundaries users actually touch:
run,upload, andtddoption precedence from CLI/config/env/git detection.uploadAll, and comparison settings.failOnDiff.One honest note: this does not add the broad visual-diff E2E matrix we still want. The current tests prove the option contract through CLI/client/server boundaries, but a follow-up should run a tiny fixture app end to end, intentionally create a visual diff, and assert the real CLI/API/dashboard outputs without mocking our own code.
Review guide
src/utils/screenshot-options.js,src/client/index.js,src/sdk/index.js, andsrc/server/routers/screenshot.jsfor the screenshot option/property contract.src/cli.js,src/utils/config-loader.js, andsrc/utils/environment-config.js.src/commands/run.js,src/commands/upload.js,src/commands/tdd.js,src/commands/api.js,src/commands/preview.js, andsrc/commands/project.js.src/uploader/*,src/test-runner/*, andsrc/services/build-manager.js.src/types/*.d.tsandtest-d/*.Verification
pnpm test,pnpm run lint,pnpm run test:types,git diff --check.node --test tests/utils/screenshot-options.test.js tests/sdk/client.test.js tests/server/routers/screenshot.test.js.node --test tests/server/handlers/api-handler.test.js tests/server/handlers/tdd-handler.test.js.CI=true pnpm run test:types.git diff --checkand PR file-list checks for ✨ Align core SDK option contracts #283 through 📝 Document aligned SDK workflows #287.