Skip to content

test(mwpw-184989): seed regressions to validate multi-page agent - #536

Open
sanrai wants to merge 13 commits into
sanrai/agent-multipagefrom
sanrai/agent-multipage-validate
Open

test(mwpw-184989): seed regressions to validate multi-page agent#536
sanrai wants to merge 13 commits into
sanrai/agent-multipagefrom
sanrai/agent-multipage-validate

Conversation

@sanrai

@sanrai sanrai commented Jun 23, 2026

Copy link
Copy Markdown
Collaborator

Throwaway validation PR. Seeds TWO deliberate regressions: (1) card titles truncated to one line (visual, should be flagged on the card pages + gallery), (2) OR filter returns zero results via tag.id->tag.name (functional, should be flagged on the left/top filter pages). The multi-page agent should run all 5 pages and flag these. Will be closed.

@github-actions

github-actions Bot commented Jun 23, 2026

Copy link
Copy Markdown

🤖 Agent QA review — multi-page, interactive + visual diff (advisory, non-blocking)

Reviewed 2 page(s) on the live site with the PR build injected. Overall: FAIL.

FAIL — A-left-hub · 0.07% pixels changed · https://business.adobe.com/customer-success-stories.html

Tested https://business.adobe.com/customer-success-stories.html with PR #536's CaaS build injected.

STEP 1 - Diff image review: The captured PR-vs-stable pixel diff showed magenta highlighting only around the top nav area; the rest of the page (including the card grid) appeared dimmed/unchanged in that snapshot, suggesting the diff tool caught the page mid-render rather than a nav-specific regression.

STEP 2 - Live inspection:

  • Top nav, breadcrumb, hero video banner, and the "Featured Adobe customers" 3-card row (Fanatics Collectibles, Workday, TSB) all rendered correctly: images loaded, headlines/descriptions intact, CTA buttons ("Read article"/"Read now") present and properly aligned. No truncation, clipping, or misalignment observed here.
  • However, scrolling to the main CaaS-driven section "Explore all Adobe customer success stories" revealed a broken state: the results counter shows "0 results" and the grid area displays "Sorry, there was a system error." instead of any cards. This is the exact collection/component the PR's Helpers/CardFilterer/getFilteredCards changes target (per the PR diff, changes to Filters/Left, Helpers, and Container.jsx affect this "A-left-hub" page).
  • I re-screenshotted after waiting (ruling out normal lazy-load blank-first-paint) and the error persisted identically on a second check, and it recurred after a full page reload as well. This is not a transient lazy-load blank state - it's a persistent broken/empty grid with an explicit system-error message, matching the failure mode the reviewer was warned about ("a Helpers/filter change can EMPTY the grid").

STEP 3 - Console errors: get_console_errors reported "Failed to load resource: net::ERR_CONNECTION_CLOSED", consistent with the CaaS data-fetch call failing, which explains the "0 results" / system-error empty state in the main filtered card grid.

Conclusion: The hero, breadcrumb, and Featured Customers cards render fine and match expectations. But the primary left-filter CaaS collection ("Explore all Adobe customer success stories") fails to load any cards and displays a persistent "Sorry, there was a system error" message with 0 results — a real, reproducible defect in the touched component (Helpers/CardFilterer/data-fetch layer), not a lazy-load artifact. This is a blocking regression for the core feature under test (left-filter hub cards, search, sort, pagination, bookmarking are all impossible to verify because the grid never populates).

Verdict: FAIL

FAIL — B-top-panel · 21.42% pixels changed · https://news.adobe.com/news?ch_News+articles=Experience%2520Cloud

Tested https://news.adobe.com/news?ch_News+articles=Experience%2520Cloud with PR #536's CaaS build injected.

STEP 1 - Diff review: The captured PR-vs-stable diff image showed a large magenta region covering essentially the entire card-grid/results area (21.42% of pixels changed), indicating the PR build renders that region completely differently from stable.

STEP 2 - Live inspection: Navigated to the target URL. Page chrome (header, nav, "Get all the latest Adobe news" hero, Filters bar showing "News Articles 1" chip, Sort dropdown, Adobe Summit promo banner, and footer) all rendered correctly and are pixel-clean/aligned. However, the card grid itself shows "No Results Found — Try checking your spelling or broadening your search." immediately on load. I waited several seconds and re-screenshotted; the empty state persisted (not a lazy-load transient — the stable/original page for this same pre-applied filter normally returns a populated grid of News Articles cards, as reflected by the large diff region).

Confirmed programmatically: document.querySelectorAll('.consonant-Card').length === 0 — zero cards rendered in the DOM. get_console_errors reported no JS errors, so this is a silent filtering failure rather than a crash.

Likely cause (per PR diff): the pre-applied top-filter URL param (ch_News+articles=Experience+Cloud) is being resolved by the CaaS filtering logic (Helpers/getFilteredCards or the Top-Filters container) to an empty result set in this build, even though the filter chip itself ("News Articles 1") correctly appears as selected. This is exactly the kind of Helpers/filter regression the PR diff warns can "EMPTY the grid."

Verdict: FAIL — the top-filter pre-applied collection on this page renders a persistent empty/broken grid (no cards) instead of the expected populated News Articles results, which is a real, visually-confirmed defect, not a lazy-load artifact.

Per-page screenshots + diffs in the workflow run.

@sanrai
sanrai force-pushed the sanrai/agent-multipage-validate branch from ae62c22 to 8032348 Compare June 24, 2026 00:04
@github-actions

Copy link
Copy Markdown

AI Code Review

Issues Found

1. Incorrect Tag Matching Logic (Correctness Bug)

File: react/src/js/components/Consonant/Helpers/Helpers.js

- const tagIds = new Set(card.tags.map(tag => tag.id));
+ const tagIds = new Set(card.tags.map(tag => tag.name));

Problem: The active filters almost certainly store filter criteria by id (not name). The code downstream checks whether tagIds contains values from the active filters. If active filters use tag.id for matching (which is standard), changing the Set to use tag.name will cause all tag-based filtering to fail silently — cards will never match filters, or filters will match incorrectly if names happen to collide.

Action: Verify what values activeFilters stores for comparison. If they store id values (the typical pattern), revert this change. If name matching is intentional, ensure activeFilters was updated consistently to also store names — check all filter-building code paths.


2. CSS Line Clamp Reduction (Potential Content Truncation / Data Loss in UI)

File: less/components/consonant/cards/card.less

- .line-clamp(2);
+ .line-clamp(1);

This is lower severity, but if card titles are expected to wrap to 2 lines, reducing to 1 will silently truncate titles in the UI. This is a functional regression if titles commonly exceed one line, potentially hiding critical content from users. Confirm this is intentional.

@sanrai
sanrai force-pushed the sanrai/agent-multipage-validate branch from 8032348 to d97c5fd Compare June 24, 2026 00:47
@github-actions

Copy link
Copy Markdown

AI Code Review

Issues Found

Critical Bug: Filter Logic Broken (Helpers.js)

Line: const tagIds = new Set(card.tags.map(tag => tag.name));

Changing tag.id to tag.name will break tag matching. The active filters almost certainly store filter values by id, not name. The comparison downstream (checking if tagIds contains filter tag identifiers) will fail silently — cards that should be filtered out will pass through, or cards that should appear will be excluded.

Action: Verify what property the active filter tags use for matching. If filters reference tag.id, this must be reverted to tag.id. If a migration to tag.name is intentional, the filter panel/active filter state must also be updated atomically in this same PR to use name as the identifier — otherwise filter functionality is broken.


Potential Issue: CSS Change (card.less)

Changing .line-clamp(2) to .line-clamp(1) truncates card titles to a single line. This is a visual regression risk — any card with a two-line title will have content silently cut off with no way for the user to see the full title. Confirm this is intentional and that truncated titles are accessible (e.g., via title attribute or tooltip).

@github-actions

Copy link
Copy Markdown

AI Code Review

Critical Issues

tag.idtag.name in getFilteredCards (Helpers.js)

Severity: High — Correctness Bug / Broken Logic

- const tagIds = new Set(card.tags.map(tag => tag.id));
+ const tagIds = new Set(card.tags.map(tag => tag.name));

Problem: Filter matching almost certainly relies on comparing tag identifiers (IDs) against filter criteria. Tag names are human-readable strings (e.g., "Adobe Photoshop") while activeFilters/activePanels would contain IDs (e.g., "caas:product/photoshop"). This change will break all filtering — cards that should match filters will no longer match, causing filtered results to return nothing or incorrect sets.

Action: Revert to tag.id unless there is explicit evidence that activeFilters stores tag names instead of IDs, and verify the downstream has() checks use the same property consistently.


line-clamp(2)line-clamp(1) (card.less, three-fourth.less)

Severity: Medium — Data Loss / UX Breakage

Reducing the clamp from 2 lines to 1 line truncates card titles to a single line. For cards with longer titles, meaningful content will be hidden with no way for users to see it. This is a functional regression — it's not a style preference, it affects readability and accessibility of card titles.

Action: Confirm this is intentional per design spec. If so, verify &:only-child max-height calculation in three-fourth.less (still set to 1.375rem * 4) is updated to match the new 1-line clamp — it's now inconsistent.

@github-actions

Copy link
Copy Markdown

AI Code Review

Critical Issues

1. Correctness Bug / Broken Filtering LogicHelpers.js

- const tagIds = new Set(card.tags.map(tag => tag.id));
+ const tagIds = new Set(card.tags.map(tag => tag.name));

This changes the filter matching key from tag.id to tag.name. The active filters (populated from activeFilters/activePanels) almost certainly store filter values by id, not name. Matching filter IDs against tag names will cause mismatches, breaking all tag-based filtering (cards will either never match or match incorrectly). This is the likely root cause of the "zero results after filtering" bug the QA step is testing for — but the fix goes the wrong direction unless the filter values were also changed to use name everywhere, which is not shown in this diff.

Verify: Confirm that activeFilters / activePanels values are populated with tag.name (not tag.id) throughout the codebase. If they still use id, this change will completely break filtering.


2. Visual Regression / Data Truncationcard.less, three-fourth.less

- .line-clamp(2);
+ .line-clamp(1);

Reducing line clamp from 2 to 1 truncates card titles and content to a single line. This is a functional regression if titles are expected to wrap (e.g., long titles silently truncated in production UI). Combined with the :only-child rule in three-fourth.less setting max-height: 1.375rem * 4, the single-line clamp is inconsistent with the max-height allowing 4 lines — the CSS becomes contradictory for the only-child case.

@github-actions

Copy link
Copy Markdown

AI Code Review

Critical Issues

1. Breaking Bug: Filter matching changed from tag.id to tag.nameHelpers.js

- const tagIds = new Set(card.tags.map(tag => tag.id));
+ const tagIds = new Set(card.tags.map(tag => tag.name));

Filter logic compares card tags against activeFilters, which are keyed by tag ID (e.g., "caas:products/acrobat"). Switching to tag.name (e.g., "Acrobat") means the set intersection will never match, causing all filtered results to return zero cards for any active filter.

This is likely the bug the new QA test in agent-review.mjs is designed to detect ("Zero or 'No results' after one normal filter is THE BUG") — but the fix goes in the wrong direction. The change introduces the bug rather than fixing it.

Action: Revert to tag.id unless activeFilters has been simultaneously changed to match by name (no such change is present in this diff).


2. Hardcoded page selection overrides all filter logicagent-review.mjs

- if (selected.length === 0) selected = [PAGES[0]];
+ selected = [PAGES[0]]; // TEMP: single-page validation (revert before scaling)

This unconditionally forces only PAGES[0] regardless of what files changed, making the CI review permanently incomplete for all other page types. If merged without reverting, PRs touching B-top-panel, C-search, etc. will never be QA-tested. The comment says "revert before scaling" but there is no enforcement mechanism.

Action: Do not merge with this hardcoded override. Revert to the conditional fallback.

@sanrai
sanrai temporarily deployed to github-pages July 9, 2026 10:24 — with GitHub Actions Inactive
@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown

AI Code Review

AI review unavailable (response parse error). See Actions logs.

@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown

AI Code Review

Critical Issues

1. Helpers.js — Filter matching broken (correctness bug)

const tagIds = new Set(card.tags.map(tag => tag.name));

This changes the set from tag IDs to tag names, but the code is presumably still matching against filter/tag IDs elsewhere (variable is still named tagIds and used for filtering logic). Unless every downstream comparison was also switched from ID-based to name-based matching, this will silently break OR/AND/XOR filtering — cards will fail to match filters they should match (or match incorrectly if names collide). This is a high-risk regression since filtering is core functionality covered by e2e tests that this PR's other changes seem to be bypassing. Verify all comparisons against tagIds use tag names consistently, or revert to tag.id.

2. card.less / three-fourth.less — line-clamp reduced from 2 to 1

.line-clamp(2);  →  .line-clamp(1);

Applied in two places. This truncates card titles to a single line instead of two, which will cut off previously-visible text. If unintentional, this is a visual regression (title truncation) that will affect many cards site-wide. Confirm this is an intended design change, not an accidental copy-paste/leftover from testing.

3. agent-review.mjs — Hardcoded page selection overrides real trigger logic

selected = PAGES.filter((p) => p.id === 'A-left-hub' || p.id === 'B-top-panel'); // TEMP: validate A+B loose-visual

This unconditionally overwrites the trigger-based/shared-file selection logic computed just above it, meaning:

  • Pages C, D, etc. (e.g., events sort/timing checks) will never be tested regardless of what files changed.
  • The Helpers.js change (which affects filtering used across multiple page types) and the .less changes will not get full coverage from pages whose triggers would normally catch them.

Given this same PR modifies Helpers.js (filter logic) and card .less files, disabling the trigger-based selection in the same PR means the QA tool won't properly validate the very changes being shipped. This looks like debug/temporary code left in — remove it or gate it behind an env flag before merging.

@sanrai
sanrai temporarily deployed to github-pages July 9, 2026 10:38 — with GitHub Actions Inactive
@sanrai
sanrai temporarily deployed to github-pages July 9, 2026 10:42 — with GitHub Actions Inactive
@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown

AI Code Review

Critical issues found:

  1. react/src/js/components/Consonant/Helpers/Helpers.js: Changing tag.id to tag.name in the tagIds Set construction is a likely correctness bug. Filters are almost certainly matched/selected by tag id elsewhere in the codebase (filter definitions, active filter state, etc.). Switching to tag.name will break filtering unless every other reference to filter tag identity was also changed to use name — if not, filters will silently stop matching, or worse, match incorrectly if names collide/differ in casing from ids. This needs verification against how activeFilters/filterType values are populated.

  2. card.less / three-fourth.less: line-clamp(2)line-clamp(1) reduces the number of visible lines for card titles from 2 to 1. This will truncate previously-visible second-line text across all cards using these styles — a visible content-loss regression, not just a style tweak, given titles will now cut off mid-word/sentence for any title that previously wrapped to 2 lines.

  3. .github/qa/agent-review.mjs:

    • selected = PAGES.filter((p) => p.id === 'A-left-hub' || p.id === 'B-top-panel'); hardcodes page selection, discarding the prior trigger-based logic entirely (including the isShared fallback and the "select all if empty" fallback). This is marked // TEMP but as committed, it disables coverage for pages C (events) and any others regardless of what the diff actually touches — a real regression in test coverage if merged as-is.
    • The VISUAL instruction explicitly tells the agent "Do NOT navigate or interact" for pages A and B, which previously had rich interactive instructions (filter toggling, pagination, bookmarking, search). This silently downgrades functional QA to visual-only for these pages, losing bug-catching coverage for filter logic, pagination URL behavior, etc. Combined with the Helpers.js tag-matching change above, this reduces the chance of catching a real filtering regression that this PR itself may introduce.

Action items:

  • Verify tag.name vs tag.id change against all call sites of activeFilters/filterType to ensure consistent key usage before merging.
  • Confirm line-clamp(1) is an intentional design change, not an accidental revert — this affects visible content on all cards.
  • Remove the hardcoded selected = PAGES.filter(...) override (or gate it behind an explicit flag) before merging so trigger-based page selection isn't permanently disabled.

@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown

AI Code Review

Critical Issues

1. Broken filtering logic — Helpers.js (correctness bug)

const tagIds = new Set(card.tags.map(tag => tag.name));

This changes the set from tag IDs to tag names, but it's being built into a variable still named tagIds and almost certainly compared against filter values that are IDs elsewhere (e.g. activeFilters/activePanels keyed by tag id). This will silently break filtering (no matches, or wrong matches) for any card whose tag name differs from its id. This looks like an unintentional/erroneous change — likely should remain tag.id. This is a functional regression and should be reverted unless there's a corresponding, verified change to how filters are keyed.

2. Truncated card titles — card.less / three-fourth.less (visible regression)

- .line-clamp(2);
+ .line-clamp(1);

Changing line-clamp from 2 to 1 in both card title styles will truncate any two-line title down to one line across the card grid and the three-fourth card. This is a significant visual/UX regression affecting all cards using these styles, not a minor style nit — titles will be cut off mid-word for the vast majority of real content.

3. Selection override disables real test coverage — agent-review.mjs

selected = PAGES.filter((p) => p.id === 'A-left-hub' || p.id === 'B-top-panel'); // TEMP: validate A+B loose-visual

This unconditionally overrides the trigger-based selection logic, meaning:

  • Pages C/D/etc. (events, other collections) are never reviewed regardless of what changed.
  • The isShared / triggers.test logic above becomes dead code.
  • If merged as-is (looks like a "TEMP" debug line left in), CI QA review silently loses coverage for all pages except A and B, for every future PR touching shared card/less files — including the card.less/three-fourth.less truncation regression above (which touches shared card styles most other collection pages also render). This should not be merged; remove the override or gate it behind an env var before merging.

4. Weakened test rigor swapped in for pages A/B

Pages A and B previously had detailed interactive instructions (exercise filters, sort, pagination, bookmarks, search) with instr covering real functional behavior. They're now switched to kind: 'visual' with an explicit instruction to not interact, relying only on screenshot diffing. Combined with issue #3, this means the two most complex, most-exercised pages (filter panels) lose all interactive/functional QA coverage in this PR, right as the Helpers.js tag-filtering bug (issue #1) is introduced — the automated agent is now structurally incapable of catching the exact class of bug this PR contains.

@sanrai
sanrai temporarily deployed to github-pages July 9, 2026 10:50 — with GitHub Actions Inactive
@sanrai
sanrai temporarily deployed to github-pages July 9, 2026 11:50 — with GitHub Actions Inactive
@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown

AI Code Review

Critical Issues

1. Helpers.js — filtering logic broken by key mismatch

const tagIds = new Set(card.tags.map(tag => tag.name));

This changes the set from tag ids to tag names, but filter matching elsewhere in the codebase almost certainly compares against activeFilters/activePanels values keyed by tag id, not name. Unless every other reference was also switched (not shown in this diff), this will break filtering — either matching nothing (if ids are used elsewhere) or silently matching wrong tags (if names collide/differ in case/whitespace from ids). This is a functional regression, not a stylistic change. Verify all call sites use name consistently, or revert.

2. card.less / three-fourth.less — line-clamp reduced from 2 to 1

.line-clamp(2); → .line-clamp(1);

Applied in two card components. This truncates card titles/text far more aggressively, which will visibly cut off headlines mid-word in many cases. Given the accompanying agent-review.mjs changes explicitly set up "loose visual" QA to catch exactly this class of regression (truncated text), this looks like an unintended change that the automated visual check is designed to flag — worth confirming this is intentional design change and not a leftover/debug edit.

3. agent-review.mjs — hardcoded override defeats trigger-based page selection

selected = PAGES.filter((p) => p.id === 'A-left-hub' || p.id === 'B-top-panel'); // TEMP: validate A+B loose-visual

This unconditionally overrides the triggers-based selection logic above it (and the isShared fallback), meaning:

  • Pages C, D, etc. will never be tested regardless of what changed.
  • This is marked // TEMP, indicating it's debug/validation code not meant for merge — if merged as-is, real regressions in non-A/B pages (e.g., the card.less/three-fourth.less changes above, which affect other card types) will be silently skipped.

Action: Remove this line before merging, or gate it behind an explicit env flag so it can't accidentally ship.

@sanrai
sanrai temporarily deployed to github-pages July 9, 2026 11:54 — with GitHub Actions Inactive
@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown

AI Code Review

Critical Issues

1. Broken filter matching logic (Helpers.js)

const tagIds = new Set(card.tags.map(tag => tag.name));

This changes the set to be built from tag.name instead of tag.id, but the variable is still named tagIds and is presumably compared elsewhere against filter IDs (not names). Unless every downstream check was also changed to compare against names, this will silently break filtering — cards will stop matching filters that rely on tag ID comparison, producing incorrect/empty result sets. This is a high-confidence correctness bug and should be verified against how tagIds is consumed later in the function (e.g., filterGroups/activeFilters comparisons, which likely use IDs).

Action: Revert to tag.id unless there's a corresponding change (not shown in this diff) that switches all downstream comparisons to use tag names consistently.

2. Unrelated/regressive CSS change (card.less, three-fourth.less)

- .line-clamp(2);
+ .line-clamp(1);

Truncating card titles from 2 lines to 1 line is a visible UI regression (more aggressive truncation, likely cutting off titles mid-word) with no accompanying explanation in the PR. Combined with the new "loose visual QA" changes in agent-review.mjs, this looks like it may be an intentional test fixture change left in accidentally, or a real regression. If unintentional, revert to 2.

3. QA script hardcoded to bypass page-selection logic (agent-review.mjs)

selected = PAGES.filter((p) => p.id === 'A-left-hub' || p.id === 'B-top-panel'); // TEMP: validate A+B loose-visual

This unconditionally overrides the trigger-based selection logic, meaning:

  • Pages C, D, etc. will never be QA'd regardless of what changed.
  • The Helpers.js change above (which affects filtering across all collection types) won't get proper interactive/functional review coverage for non-A/B pages.

Marked TEMP — if merged as-is, this silently disables real CI coverage for other page types. Must be removed/reverted before merge.

@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown

AI Code Review

Critical Issues

1. Helpers.js — Filter logic bug (correctness)

const tagIds = new Set(card.tags.map(tag => tag.name));

This changes the set from tag.id to tag.name. If filter matching elsewhere (e.g., activeFilters ids) still compares against tag IDs, this will break filtering entirely — cards will fail to match filters that previously worked, or match incorrectly if names collide/differ from ids. This looks like an unintentional/leftover change (possibly from a search-replace or merge error) and should be reverted unless the entire filter-matching contract was also changed to use names consistently. Verify all callers/comparisons use name consistently, otherwise revert to tag.id.

2. card.less / three-fourth.less — Line-clamp reduced from 2 to 1

.line-clamp(2); → .line-clameta(1);

Applied in two separate card components. This truncates titles/text to a single line, which will visibly cut off previously-visible second lines of card titles across the site. Given the PR's own QA tooling explicitly calls out "truncated/clipped text" as a regression to catch, this is very likely an unintended regression rather than a deliberate design change. Confirm this is intentional; if not, revert to .line-clamp(2).

3. agent-review.mjs — Hardcoded page selection overrides trigger-based logic

selected = PAGES.filter((p) => p.id === 'A-left-hub' || p.id === 'B-top-panel'); // TEMP: validate A+B loose-visual

This unconditionally overrides the triggers-based selection (and the isShared fallback), meaning pages C, D, etc. will never be tested, regardless of what changed. Marked // TEMP, but if merged as-is, this permanently disables QA coverage for all other collection pages (events, etc.) — silently reducing test coverage with no visibility that it happened. This must be removed/reverted before merge, since it directly contradicts the surrounding trigger-detection logic and will mask regressions in untested pages (including the real regressions above, e.g. events sort, that this same PR could affect).

@sanrai
sanrai temporarily deployed to github-pages July 9, 2026 12:24 — with GitHub Actions Inactive
@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown

AI Code Review

AI review unavailable (response parse error). See Actions logs.

@sanrai
sanrai temporarily deployed to github-pages July 9, 2026 20:47 — with GitHub Actions Inactive
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.

1 participant