feat(core): tip about VRA when a build uses Layout review mode (PER-9502)

[prklm10]

Summary

• Emits a single warn-level tip immediately before the Finalized build log whenever any snapshot in a build had Layout review mode enabled (enableLayout, set via global config or per-snapshot).
• Why: Layout is a legacy review mode; VRA (Visual Review Agent) is the recommended replacement. This surfaces a contextual nudge at build finalization — where users already look — with minimal noise (one line, only when Layout is actually used).
• Key decisions:
  • Detection flips a per-build flag (build.layoutUsed) in the queue push handler; emission is gated in the end handler, so it logs at most once per process.
  • Covers both standard and parallel builds: each parallel shard runs core's end handler, so cli-build finalize needs no change.
  • Tip copy (renders with the [percy] prefix at normal log level):

    Tip: VRA is Percy's recommended visual review mode — more accurate and adaptable than Layout. Learn more: https://www.browserstack.com/docs/percy/ai-agents/visual-review-agent/overview.

Changes

• packages/core/src/snapshot.js — detection in push handler + guarded warn in end handler.
• packages/core/test/snapshot.test.js — 4 new specs.

Testing

• 4 new specs in snapshot.test.js, all passing:
  • logs the tip before finalizing when a snapshot has layout enabled
  • logs the tip when enableLayout is set globally in config
  • logs the tip only once when multiple snapshots have layout enabled
  • does not log the tip when no snapshot has layout enabled
• Lint clean (eslint from repo root).
• Note: the full local core suite shows pre-existing, environment-only failures unrelated to this change — install.test.js (global.__MOCK_IMPORTS__ undefined; ESM loader hook) and one api.test.js server-disabled spec (AggregateError vs ECONNREFUSED, a Node networking quirk). No snapshot/finalize/layout specs fail.

Post-Deploy Monitoring & Validation

• What to monitor/search
  • Logs: CLI build output for the new line Tip: VRA is Percy's recommended visual review mode.
• Validation checks (queries/commands)
  • npx percy snapshot with enableLayout: true (global or per-snapshot) → tip appears once just before Finalized build.
  • Same run without any Layout snapshot → tip does NOT appear.
• Expected healthy behavior
  • Exactly one tip line per build process when Layout is used; none otherwise. Build finalization behavior unchanged.
• Failure signal(s) / rollback trigger
  • Tip appearing on builds with no Layout usage, duplicate tips within a single process, or any change to Finalized build ordering → revert this commit (log-only change, safe to revert).
• Validation window & owner
  • Window: first few CLI releases / dogfood builds after merge. Owner: PR author.

---

🤖 Generated with Claude Code

---

Update: instrumentation of the recommendation

The VRA tip is now also instrumented via a build event so we can measure how often the recommendation is shown:

• Extended PercyClient#sendBuildEvents to optionally accept the newer top-level params event_name and category (backward compatible — existing callers unchanged; when omitted the API applies its defaults).
• When the tip is shown, core sends a build event before finalizing the build (so the build is still open), wrapped in try/catch so telemetry can never fail a build:
  • event_name: percy_cli_vra_recommendation_emitted
  • category: percy:cli
  • data: { message: "VRA recommendation shown for a build using Layout review mode" }

Note: category was set to percy:cli (the CLI-side sibling of the percy:app example) since it wasn't specified — easy to change if a different taxonomy is preferred.

Added tests: client sendBuildEvents with event_name/category, and core specs asserting the event is sent (filtered by event_name) only when Layout is used.

[prklm10]

Claude Code Review (automated) — 2 inline finding(s). Full report in the PR comment below. Verdict: Passed.

[prklm10]

[Low] Tip emitted at WARN level

This is a promotional/informational tip, but it's logged via percy.log.warn. WARN normally signals something went wrong, and CI log processors or --fail-on-warning-style tooling may surface a benign mode-recommendation as a warning.

Suggestion: Use percy.log.info (or a dedicated tip level) unless WARN-level visibility is an explicit product requirement — in which case a one-line comment noting that intent would prevent future churn.

Reviewer: stack:code-reviewer

[prklm10]

Not applicable, it should be warning,

[prklm10]

[Medium] Telemetry duplicates centralized pattern and omits cliVersion/clientInfo

This swallow-and-debug-log telemetry call reimplements the pattern already centralized in percy.sendCacheTelemetry (packages/core/src/percy.js:484). Unlike that path, the event body here omits cliVersion and clientInfo, which existing build events carry for attribution.

Suggestion: Route through a shared telemetry helper, or add cliVersion: percy.client.cliVersion and clientInfo: percy.clientInfo to the body to match the established event shape. At minimum, confirm with the events consumer that the slimmer payload is acceptable.

Reviewer: stack:code-reviewer

[prklm10]

Not applicable here

[prklm10]

Claude Code PR Review

PR: #2276 • Head: e08ba27 • Reviewers: stack:code-reviewer (came to rest mid-investigation — no final report) + orchestrator source analysis

Summary

Adds a one-time "use VRA instead of Layout" tip, logged when a build that used Layout review mode is finalized, and instruments that recommendation via the send-events API. PercyClient.sendBuildEvents gains optional eventName/category params (sent as event_name/category); createSnapshotsQueue tracks build.layoutUsed on push and, at build end, logs the tip and fires a telemetry event whose failure is swallowed so it can never fail the build. A .gitignore cleanup adds .harness-manifest.json and reorders an existing entry.

Review Table

Priority	Category	Check	Status	Notes
High	Security	No hardcoded secrets or credentials	Pass	No secrets; only a public docs URL.
High	Security	Authentication/authorization checks present	N/A	No auth surface touched.
High	Security	Input validation and sanitization	Pass	validateId('build', buildId) retained; new params spread only when truthy.
High	Security	No IDOR — resource ownership validated	N/A	Build id flows from existing build context.
High	Security	No SQL injection (parameterized queries)	N/A	No DB access.
High	Correctness	Logic is correct, handles edge cases	Pass	build is shared across queue handlers; layoutUsed set on push (incl. global config via getSnapshotOptions merge) and read at end. Verified in source.
High	Correctness	Error handling is explicit, no swallowed exceptions	Pass	Telemetry intentionally swallowed + debug-logged; finalize still runs. Deliberate and tested.
High	Correctness	No race conditions or concurrency issues	Pass	layoutUsed is a monotonic boolean flag; concurrent pushes only ever set it true.
Medium	Testing	New code has corresponding tests	Pass	5 new core tests + 1 client test cover happy path, global config, once-only, telemetry-failure, and negative case.
Medium	Testing	Error paths and edge cases tested	Pass	Telemetry-rejection path explicitly covered.
Medium	Testing	Existing tests still pass (no regressions)	Pass (not run)	sendBuildEvents signature is backward-compatible (4th arg optional, defaulted).
Medium	Performance	No N+1 queries or unbounded data fetching	Pass	Single event per build at finalize.
Medium	Performance	Long-running tasks use background jobs	N/A	n/a.
Medium	Quality	Follows existing codebase patterns	Fail	Reimplements the swallow-telemetry pattern inline instead of reusing the centralized percy.sendCacheTelemetry egress (percy.js:484); also omits the cliVersion/clientInfo that existing telemetry events carry.
Medium	Quality	Changes are focused (single concern)	Pass	.gitignore reorg is unrelated but trivial.
Low	Quality	Meaningful names, no dead code	Pass	Clear names; no dead code.
Low	Quality	Comments explain why, not what	Pass	Comments justify optional params and swallow.
Low	Quality	No unnecessary dependencies added	Pass	None added.

Findings

• File: packages/core/src/snapshot.js:431

• Severity: Low

• Reviewer: orchestrator source analysis

• Issue: A promotional/informational tip is emitted at percy.log.warn level. WARN is for things that went wrong; a recommendation to switch review modes is not a warning and may be surfaced as a warning by CI log processors or --fail-on-warning-style tooling.

• Suggestion: Consider percy.log.info (or a dedicated tip level) unless WARN-level visibility is an explicit product requirement. If WARN is intentional for visibility, a one-line comment saying so would prevent future "wrong log level" churn.

• File: packages/core/src/snapshot.js:435

• Severity: Medium

• Reviewer: orchestrator source analysis

• Issue: This telemetry call duplicates the swallow-and-debug-log pattern already centralized in percy.sendCacheTelemetry (packages/core/src/percy.js:484), and unlike that path it omits cliVersion and clientInfo from the event body. Backend consumers of build events generally rely on those fields for attribution; the VRA event will be missing them.

• Suggestion: Either route this through a shared telemetry helper, or include cliVersion: percy.client.cliVersion and clientInfo: percy.clientInfo in the body to match the established event shape. At minimum confirm with the events consumer that the slimmer payload is acceptable.

• File: .gitignore (managed block)

• Severity: Low

• Reviewer: orchestrator source analysis

• Issue: .harness-manifest.json is added inside the # bstack-ai-harness:begin … :end block, which is annotated "managed — do not edit between markers." A manual edit there can be clobbered the next time the harness tool regenerates the block.

• Suggestion: Verify the entry is added by the harness tooling itself (regenerated output) rather than hand-edited, or place it outside the managed markers if it must be hand-maintained.

⚠️ stack:code-reviewer came to rest mid-investigation and returned no final findings report; the orchestrator supplemented with direct source verification of the same files. One point the agent did confirm before stopping: getSnapshotOptions merges config.snapshot (including enableLayout) into each snapshot before the push handler, so the global-config tip path is correct.

---

Verdict: PASS — Backward-compatible, well-tested change with no Critical/High issues. The Medium quality finding (telemetry pattern/field consistency) is worth addressing but is non-blocking.