Add --viewport desktop/mobile/both with back-compatible scenario keys

[LegalMarc]

Goal

Add --viewport {desktop,mobile,both} and run the scenario set under each selected viewport. CMPs frequently ship a different banner, layer count and symmetry on mobile, and that is where most traffic is — it is the largest coverage gap in the tool. A default (desktop) run must remain structurally identical to today.

Dependencies

• needs Add scenario key, plan and viewport-collapse pure functions #10

Scope

• cookie-banner-auditor/scripts/lib/capture.py:
  • A ViewportProfile dataclass (label, width, height, user_agent, is_mobile, has_touch, device_scale_factor).
  • run_all_scenarios becomes a thin executor over checks.plan_scenarios(...), using dataclasses.replace(config, viewport=...) per profile.
  • The persistence entry keeps its wrapper. plan_scenarios treats persistence as a plain action=none entry, but the executor must still wrap that entry's raw scenario result in today's exact shape — {ran, banner_reprompted, banner_text, scenario_result, note} — and store it under the entry's key (persistence, persistence@mobile). Do not flatten it to a bare scenario result. is_scenario returns True iff "checkpoints" in result; the wrapper nests checkpoints under scenario_result, which is the only reason persistence stays out of scenario_validity_map and the row filters. Flattening would put a top-level checkpoints key on it and pull it into both — which is what the out-of-scope note below means by "your key scheme is wrong" — and would silently kill the E4 finding, since analysis.py reads persistence.get("ran").
  • context_options (~1519) must set is_mobile, has_touch and device_scale_factor from the profile, not just viewport.
  • Per-scenario directory/HAR names go through checks.scenario_dir_name.
  • The banner-crop clamp (~933) currently hardcodes min(width, 1440); derive it from the actual viewport.
• cookie-banner-auditor/scripts/lib/analysis.py: split finding generation into a per-viewport pass; every literal "denial" / "baseline" / "gpc" becomes checks.scenario_key(base, vp), including the row filters and every depends_on_scenarios. Append the viewport to id_parts only when not desktop. Call checks.collapse_viewport_findings after partition_findings. Add a viewport column to the scenario-results table and label screenshots — only when more than one viewport ran.
• cookie-banner-auditor/scripts/audit_site.py: the flag (default desktop); run_detect_only (~108) honours it; metadata["viewport"] becomes "1440x1000" for desktop-only or "1440x1000+390x844" for both.
• cookie-banner-auditor/scripts/compare_runs.py: in _endpoints, skip requests whose phase is denial_verification_reload — the autosave probe's own reload traffic, landed by Wire verified-autosave denial path into execute_denial #7 (75b53da). That is the harness's traffic, not the site's, and counting it makes two runs look like they contacted different endpoints. Do not add a new phase; denial_verification_reload already exists in capture.py. Also make _comparability skip a field when both sides are None.
• Tests in scripts/tests/smoke_test.py.

Out of scope

• Bumping schema_version. No field is renamed or removed; a desktop-only run is byte-identical and a both run only adds keys (data, not schema).
• Changing is_scenario, scenario_validity_map or partition_findings — with composite keys they need no change. If you find yourself editing them, your key scheme is wrong.
• Running baseline repeats under mobile (keep them desktop-only).
• run_fingerprint — it already hashes metadata["viewport"], so encoding the set into that existing string is what keeps fingerprints correct with no change.

Acceptance criteria

• Lock test (the hard gate). A --viewport desktop run over the existing synthetic fixture produces: scenario-validity keys exactly ["baseline","denial","gpc"]; no finding id containing @ or ending -MOBILE; schema_version == "2.0"; and the exact set of top-level keys in audit-data.json equal to a hardcoded list. Assert the key set, not a subset — that is the byte-identical claim in executable form.
• A two-viewport fixture yields both denial and denial@mobile in scenario_results and scenario_validity, and a mobile-only finding whose id ends -MOBILE.
• Gating across viewports: with denial valid and denial@mobile invalid, the desktop F-POST-DENIAL-TRACKING is emitted while its mobile counterpart is suppressed, and overall status is INCOMPLETE. This is the load-bearing test — a mobile failure must never suppress a valid desktop finding, and must never let a mobile finding through.
• Identical desktop/mobile findings collapse to one row citing both viewports; findings differing in severity or evidence strength stay separate.
• The viewport column and mobile screenshot labels appear only when more than one viewport ran; a desktop-only report is unchanged.
• --viewport mobile sets is_mobile/has_touch/a mobile UA on the context. Assert the context options, not merely the width — a narrow viewport with a desktop UA gets you a squashed desktop banner, not the mobile CMP, and that failure is invisible from width alone.
• All existing tests pass unmodified.
• bash scripts/check.sh passes.

Required verification

• bash scripts/check.sh
• .venv/bin/python -c "import sys; sys.path.insert(0,'cookie-banner-auditor/scripts'); import subprocess; h=subprocess.run(['.venv/bin/python','cookie-banner-auditor/scripts/audit_site.py','--help'],capture_output=True,text=True).stdout; assert '--viewport' in h, h[:400]; print('--viewport flag present')"
• .venv/bin/python -c "import sys; sys.path.insert(0,'cookie-banner-auditor/scripts'); from lib import checks as c; assert c.scenario_key('denial','desktop')=='denial'; print('desktop keys still bare')"

Notes

• This is the riskiest ticket in the queue. Its safety rests entirely on one invariant: scenario_key(base,"desktop") == base. Route every key construction through checks.scenario_key / parse_scenario_key; never write f"{base}@{vp}" inline. A surviving == "denial" comparison fails safe (that check silently becomes desktop-only) but fails quietly, which is why the lock test and the gating test both exist.
• Finding ids must be distinct per viewport: compare_runs._findings_by_id is a dict comprehension, so two findings sharing an id silently drop one. _stable_id already normalises non-alphanumerics, so "denial@mobile" becomes -DENIAL-MOBILE on its own.
• Collapse runs after partition so that merging a mobile observation into a desktop finding can never smuggle in evidence from an unverified interaction. Do not reorder these.
• Runtime roughly doubles for both (~30 min on the thorough profile). Baseline repeats stay desktop-only, so mobile findings carry no stability caveat — note that in the report's limitations rather than leaving a reader to assume otherwise.
• Mobile emulation is not a mobile device: no real touch hardware, no real network conditions, no in-app webview. Add that to the recorded limitations.
• Update references/data-schema.md to document that scenario_results keys are base or base@viewport, since compare_runs.py now reads that contract.
• Governing invariant throughout: never emit a finding from a scenario whose interaction did not complete and verify.

[LegalMarc]

Removing afk — the coder declined to guess rather than risk a silent-failure regression, which is the right call on a ticket the spec itself calls the riskiest in the queue.

Two unresolved spec ambiguities, both need an answer from whoever wrote the ticket:

1. compare_runs.py's fix instruction references a request phase named probe (to skip probe-phase requests in _endpoints), but no such phase exists anywhere in the codebase's phase taxonomy (context_created, initial_navigation, pre_interaction, denial_interaction, post_denial, accept_interaction, post_accept, refresh, internal_navigation_N, exercise_search/forms, post_exercise) — and adding one isn't in capture.py's scope for this ticket. Is probe a new phase this ticket should introduce, a typo for an existing phase, or should that line be dropped?

2. The persistence-check wrapper shape is ambiguous under the new plan_scenarios-based executor. Today run_persistence_check wraps its result as {ran, banner_reprompted, banner_text, scenario_result, note}, and generate_findings's E4 check plus the report both depend on that wrapper. The ticket has plan_scenarios treat persistence as a plain action=none entry like any other scenario, but doesn't say whether the new thin executor preserves that wrapper per-entry or flattens it. Guessing wrong would silently break persistence-across-session findings on every run, including desktop-only — not a loud failure, exactly the kind of thing this skill's design is supposed to prevent.

No code was written — the coder stopped at the design stage, so the working tree is untouched. Restoring afk once these are clarified.

[LegalMarc]

Both ambiguities resolved and folded into the ticket body, so an AFK coder reads the corrected spec rather than having to find it in the thread. Restoring afk.

1. probe phase → denial_verification_reload. Not a typo and not a new phase: the ticket was written while #7 was queued and was forward-referencing #7's autosave probe. That phase now exists on main (75b53da) — probe_autosave stamps its own reload traffic denial_verification_reload so it can't be misread as post-denial tracking. _endpoints should skip it for the same reason: it is the harness's traffic, not the site's, so counting it makes two runs look like they contacted different endpoints. Scope note stands — nothing to add in capture.py.

2. Persistence keeps its wrapper, per entry. This one is settled by the ticket's own out-of-scope clause rather than by preference. is_scenario returns True iff "checkpoints" in result, and the wrapper nests checkpoints under scenario_result — that nesting is the only thing keeping persistence out of scenario_validity_map and the row filters. Flattening gives it a top-level checkpoints key, pulls it into both, and forces exactly the edits the ticket says mean "your key scheme is wrong". It would also silently kill the E4 finding (analysis.py reads persistence.get("ran")) and break the byte-identical lock test, since audit-data.json's persistence value would change shape. So: the executor runs the plan entry, then wraps the result in today's exact shape under the entry's key (persistence, persistence@mobile).

Both edits are in the body now — Scope's compare_runs.py line, and a new sub-bullet under the capture.py thin-executor item.

[LegalMarc]

Pulling afk again — not for the two spec ambiguities (those are resolved and folded into the body), but for something bigger I hit while prepping the queue: this ticket's goal has already shipped, under a different architecture, and the ticket would replace it. That is an owner's call, not a coder's.

What's already on main

66fa4b0 — "audit mobile as well as desktop" (Aug 21) — already landed --viewport {desktop,mobile,both}, VIEWPORT_PROFILES, the full device emulation (is_mobile/has_touch/device_scale_factor/mobile UA), build_context_options as an assertable seam, and 4 tests. Both of this ticket's Required verification commands already pass on unmodified main:

AC: --viewport flag present
AC: desktop keys still bare

The conflict

The two designs solve the same problem in incompatible ways:

	66fa4b0 (shipped)	#11 (specified)
--viewport both output	two complete bundles under desktop/ and mobile/	one bundle, composite keys denial@mobile
findings	never merged; kept per-bundle so a mobile and a desktop observation can't be conflated	collapse_viewport_findings merges identical ones into one row citing both
gate signals	unioned across profiles, invalid scenarios qualified by profile name	single partition_findings pass over composite keys
metadata["viewport"]	per-bundle, "1440x1000" / "412x915"	composite, "1440x1000+390x844"

#10 (a039690, landed the day after 66fa4b0) added scenario_key, parse_scenario_key, plan_scenarios, scenario_dir_name and collapse_viewport_findings as pure functions for the #11 design. I checked: they have zero production callers. grep across audit_site.py, compare_runs.py, capture.py and analysis.py finds nothing — they are exercised only by tests, exactly as plan_scenarios' own docstring says ("Nothing calls this yet").

So the real scope of #11 is not "add mobile" — it is rip out a shipped, tested, working feature and rebuild it around composite keys. That is a much riskier ticket than the body describes, on a ticket the body already calls the riskiest in the queue, and I don't think an AFK coder with clean context should be pointed at it: it would most likely not even notice 66fa4b0 exists and would produce a second, parallel mobile implementation.

Other body inaccuracies this uncovered

• Goal — "Add --viewport {desktop,mobile,both}" — already added.
• Scope — "A ViewportProfile dataclass" — exists as VIEWPORT_PROFILES plus ScenarioConfig's emulation fields.
• Scope — "context_options (~1519) must set is_mobile, has_touch, device_scale_factor" — done, in build_context_options.
• Scope — "the banner-crop clamp (~933) hardcodes min(width, 1440)" — worth re-checking against current line numbers; the file has moved a lot since.
• AC — "1440x1000+390x844" hardcodes a 390x844 mobile profile, but the landed profile is 412x915 (Pixel-7-class, chosen deliberately — see the VIEWPORT_PROFILES comment). Any rewrite should derive this from VIEWPORT_PROFILES, never hardcode it.
• AC — "All existing tests pass unmodified" is unsatisfiable as written: 66fa4b0's 4 viewport tests assert the two-bundle layout this ticket removes.

The decision owed

1. Keep 66fa4b0's two-bundle design → close Add --viewport desktop/mobile/both with back-compatible scenario keys #11, and either delete Add scenario key, plan and viewport-collapse pure functions #10's five unused functions or open a small ticket to do so. Cheapest, and the shipped design has a real argument behind it (a mobile and a desktop observation are never conflated in one table).
2. Migrate to Add --viewport desktop/mobile/both with back-compatible scenario keys #11's composite-key design → Add --viewport desktop/mobile/both with back-compatible scenario keys #11 needs a rewrite as a migration ticket: what happens to the desktop/+mobile/ layout, whether that's a breaking change to bundle paths, whether schema_version still stays at 2.0 (the out-of-scope note assumes a pure addition, which a layout change is not), and what happens to 66fa4b0's tests.
3. Keep both → doesn't seem coherent; two mechanisms for one flag.

I'd lean (1) — the shipped design is working, tested, and its per-bundle separation is defensible on exactly the grounds this project cares about — but it's your call, so I'm not acting on it. #8 is unaffected and proceeding now.

The two earlier clarifications remain correct and are still in the body; they'd apply as-is if you pick (2).

[LegalMarc]

Moving to backlog — mobile is deferred while we finish the quality pass on what already works.

Nothing here is lost. The two spec clarifications are folded into the body, and the architecture conflict is documented in the comment above: the goal already shipped in 66fa4b0, so whenever this comes back it should come back as a migration ticket (two-bundle → composite-key), not as a feature ticket. The body's Goal/Scope sections still describe it as the latter and will need rewriting first.

The five unused pure functions from #10 (scenario_key, parse_scenario_key, plan_scenarios, scenario_dir_name, collapse_viewport_findings) stay on main as dead code while this sits in the backlog. They're tested and documented as not-yet-called, so they're harmless, but they are the sole reason those tests exist — worth revisiting if this stays deferred.