Skip to content

Fix/consistent prompting#2

Merged
Hugoer merged 10 commits intomainfrom
fix/consistent-prompting
Apr 9, 2026
Merged

Fix/consistent prompting#2
Hugoer merged 10 commits intomainfrom
fix/consistent-prompting

Conversation

@Hugoer
Copy link
Copy Markdown
Owner

@Hugoer Hugoer commented Apr 9, 2026

Description

Remove the if (options.profile || options.network || options.device) { ... return resolved; } block (lines 246–257). Instead, restructure so that:

  1. Profile/network/device resolution remains at the top (lines 246–254) but does NOT return early. It simply populates resolved.runs and falls through.
  2. If resolved.runs is still empty after the flag check (no profile/network/device flags), THEN prompt interactively for profiles (the existing interactive block at lines 260–314).
  3. skipAudits prompt (lines 316–333) always runs — it already checks if (!options.skipAudits).
  4. blockedUrlPatterns prompt (lines 335–347) always runs — it already checks if (!options.blockedUrlPatterns).
  5. stripJsonProps prompt (lines 349–361) always runs — it already checks if (options.stripJsonProps === undefined).

The key change is simply removing return resolved; at line 256 and wrapping the interactive profile section in a conditional on resolved.runs.length === 0.

Success Criteria

  1. web-perf lab --profile=low (no URL) prompts for: URL, skipAudits, blockedUrlPatterns, stripJsonProps
  2. web-perf lab --profile=low https://example.com prompts for: skipAudits, blockedUrlPatterns, stripJsonProps
  3. web-perf lab --profile=low --skip-audits=full-page-screenshot https://example.com prompts for: blockedUrlPatterns, stripJsonProps
  4. web-perf lab (no flags) still works identically to today (full wizard)
  5. All existing tests pass
  6. New tests cover the flag-path scenarios

Type of Change

  • feat: A new feature
  • fix: A bug fix
  • perf: A code change that improves performance
  • refactor: A code change that neither fixes a bug nor adds a feature
  • test: Adding missing tests or correcting existing tests
  • build: Changes to the build system, dependencies, or packaging tools
  • ci: Changes to CI configuration or scripts
  • docs: Documentation changes only

Scope

  • lab: Lighthouse lab audits (local, headless Chrome)
  • rum: PageSpeed Insights API (RUM data)
  • collect: CrUX API data collection
  • sitemap: Sitemap.xml parsing and URL extraction
  • links: Internal link extraction from rendered DOM
  • cli: CLI entrypoint and argument parsing
  • prompts: User prompts and interactive flows
  • profiles: Device/network simulation profiles
  • utils: Shared utilities and helpers

Related Issue

Fixes #1

Checklist

  • My code follows the code style of this project.
  • I have performed a self-review of my own code.
  • I have read the CONTRIBUTING document.
  • (If applicable) I have updated the documentation.

Hugoer and others added 10 commits April 9, 2026 13:26
Shallow removal of root-level keys from a JSON object.
Designed for future deep/recursive extension.
No project dependencies — standalone CommonJS module.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… write

When labOptions.stripJsonProps !== false (default), removes root-level
'i18n' and 'timing' keys via stripJsonProps before writing the file.
--no-strip-json-props disables stripping (raw Lighthouse report).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Interactive mode: 'Allow strip unneeded properties? (Y/n)' confirm
prompt appears after blocked-url-patterns, default true.

CLI-flag path: propagates options.stripJsonProps = false to resolved
when --no-strip-json-props is passed. Default (true) is a no-op on
the flag path since lab.js treats undefined/true identically.

Updated all 10 affected interactive tests with the new 4th mock.
Added 3 new tests: confirm true, confirm false, CLI flag false path.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Uses Commander's --no-* negation pattern: options.stripJsonProps is
true by default, false when --no-strip-json-props is passed.

Wires stripJsonProps through labAction into every runLab() call,
resolving from promptLab result first, then Commander options.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…before-define

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1. prompts.js — always set resolved.stripJsonProps in CLI-flag path
   (was only set when false, leaving true case implicit). Symmetric
   with interactive path. Fallback in labAction simplified to ??.

2. lab.js — preserve result.report write when stripping is disabled.
   Avoids unnecessary JSON.parse + JSON.stringify on large files
   (~20 MB) when --no-strip-json-props is passed, and keeps original
   byte-for-byte output for that path.

3. prompts.test.js — add test covering stripJsonProps: true via CLI
   flag path to complement the existing false test.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Root cause: Commander's --no-strip-json-props negation pattern sets
options.stripJsonProps = true by default, so promptLab's condition
(options.stripJsonProps === undefined) was never true, silently
skipping the interactive confirm prompt.

Fix: labAction uses cmd.getOptionValueSource('stripJsonProps') to
distinguish a user-explicit CLI flag ('cli') from Commander's default
('default'). Passes undefined to promptLab when it's the default, so
the interactive prompt fires correctly.

Regression test: 'should prompt for stripJsonProps when
options.stripJsonProps is undefined' — fails without the fix.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
ADR-001 captures the architectural decision:
- Why strip i18n and timing properties (reduce file size, unused metadata)
- Why opt-out (default useful for most users)
- Why shallow (vs deep, vs compression, vs configurable)
- Bug found/fixed: Commander flag-source detection for interactive prompt

README updates:
- Add --no-strip-json-props flag to lab command examples
- Document in parameter table with link to ADR-001
- Update command summary table

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- promptLab: remove early return when --profile/--network/--device flags
  are given; skipAudits, blockedUrlPatterns, and stripJsonProps are now
  prompted individually if not provided via flag
- promptSitemap: remove unconditional assertTTY() at top; move TTY guard
  inside each missing-value block; non-TTY defaults to depth=3, delay=none,
  outputAi=false; throws only when URL is missing in non-TTY
- promptLinks: same fix as promptSitemap; non-TTY with URL provided no
  longer throws
- promptPsi: categories block now falls through to undefined (all) in
  non-TTY instead of throwing when --category flag is absent

All commands now follow the rule: each option is prompted individually
if and only if it was not provided via a CLI flag (and session is TTY).

Tests: 231 passing (10 new regression tests added)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@Hugoer Hugoer merged commit ec3f922 into main Apr 9, 2026
3 checks passed
@Hugoer Hugoer deleted the fix/consistent-prompting branch April 10, 2026 11:48
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.

[BUG] Inconsistent interactive prompting across lab, sitemap, links, and psi commands

1 participant