From a01a4e42ee8a0b2260c73dd870c34fef750755c4 Mon Sep 17 00:00:00 2001 From: wilfordgrimley <2397930+WilfordGrimley@users.noreply.github.com> Date: Sat, 8 Aug 2026 22:57:54 +0000 Subject: [PATCH 1/2] feat(question-feed): de-hardcode the fixed question ladder The stage machine assumed a fixed level1 -> level2 -> level3 sequence: the suggested printing was asked about twice (once in its level-1 slot, again as a highlighted tile in level-2's grid), every non-yes level-1 answer funneled to level 2 by force, and "Art matches, not an official printing" was a level-2-only escape hatch despite being a direct resolution for the exact case level 1 was built around. The feed now has a single candidate question: the suggested printing (when present) is judged once in its own slot, the rest of the candidates sit in the grid on the same page, and the bottom row resolves the whole question (none of these / custom art / skip). Answers resolve or advance by what they mean, not by a hardcoded sequence. The only remaining "stage" is the data-driven post-selection attribute confirmation. Test selectors follow the renamed surface (question-feed-suggestion-*, question-feed-candidate-question), including the embed and responsive specs that keyed off the old level-1 testids. --- .../questionFeed/QuestionFeed.test.tsx | 39 +- .../features/questionFeed/QuestionFeed.tsx | 334 +++++++++--------- .../tests/CardbackPdfWaitFidelity.spec.ts | 6 +- frontend/tests/PDFWaitExperience.spec.ts | 11 +- frontend/tests/QuestionFeed.spec.ts | 115 +++--- frontend/tests/QuestionFeedResponsive.spec.ts | 27 +- 6 files changed, 279 insertions(+), 253 deletions(-) diff --git a/frontend/src/features/questionFeed/QuestionFeed.test.tsx b/frontend/src/features/questionFeed/QuestionFeed.test.tsx index bbef4bc35..94a6be7fb 100644 --- a/frontend/src/features/questionFeed/QuestionFeed.test.tsx +++ b/frontend/src/features/questionFeed/QuestionFeed.test.tsx @@ -385,7 +385,7 @@ describe("QuestionFeed", () => { ).toHaveTextContent("Suggested match"); }); - it("shows the suggested printing's own reference image on Level 1 (regression: dropped when Level 1 was introduced in #49)", async () => { + it("shows the suggested printing's own reference image on the suggested-match question (regression: dropped when the suggestion slot was introduced in #49)", async () => { server.use( http.get(buildRoute("2/questionFeed/"), () => HttpResponse.json( @@ -410,7 +410,7 @@ describe("QuestionFeed", () => { await revealCard(); const referenceImage = within( - await screen.findByTestId("question-feed-level1-reference-image") + await screen.findByTestId("question-feed-suggestion-reference-image") ).getByRole("img"); expect(referenceImage).toHaveAttribute( "src", @@ -1056,8 +1056,28 @@ describe("QuestionFeed", () => { ); } - it("tapping Level 1 'Not sure' POSTs an abstention for this card and question type, then advances to Level 2", async () => { - server.use(serveConfirmSuggestionOnce()); + it("tapping the suggested-match 'Not sure' POSTs an abstention for this card and question type, then advances to the next question", async () => { + // The de-laddered flow (issue #728): "Not sure" means "I can't resolve this" - it + // records the abstention (issue #712) and advances to the next question rather than + // falling into a level1 -> level2 re-ask of the same candidates. + let feedFetchCount = 0; + server.use( + http.get(buildRoute("2/questionFeed/"), () => { + feedFetchCount += 1; + return HttpResponse.json( + { + item: confirmSuggestionItem, + remainingEstimate: { + total: 1, + confirmable: 1, + contested: 0, + fresh: 0, + }, + }, + { status: 200 } + ); + }) + ); let abstentionBody: Record | undefined; server.use( http.post( @@ -1072,7 +1092,7 @@ describe("QuestionFeed", () => { await revealCard(); fireEvent.click( - await screen.findByTestId("question-feed-level1-not-sure") + await screen.findByTestId("question-feed-suggestion-not-sure") ); await waitFor(() => expect(abstentionBody).toBeDefined()); @@ -1080,12 +1100,11 @@ describe("QuestionFeed", () => { identifier: confirmSuggestionItem.card.identifier, questionType: "confirm_suggestion", }); - expect( - await screen.findByTestId("question-feed-level2") - ).toBeInTheDocument(); + // advances to the next question - no level2 re-ask of the same candidates + await waitFor(() => expect(feedFetchCount).toBe(2)); }); - it("tapping Level 1 'Skip' never calls submitQuestionAbstention", async () => { + it("tapping 'Skip' never calls submitQuestionAbstention", async () => { server.use(serveConfirmSuggestionOnce()); let abstentionCalls = 0; server.use( @@ -1097,7 +1116,7 @@ describe("QuestionFeed", () => { renderFeed(); await revealCard(); - fireEvent.click(await screen.findByTestId("question-feed-level1-skip")); + fireEvent.click(await screen.findByTestId("question-feed-skip")); await revealCard(); expect(abstentionCalls).toBe(0); diff --git a/frontend/src/features/questionFeed/QuestionFeed.tsx b/frontend/src/features/questionFeed/QuestionFeed.tsx index e85943e28..2280248bb 100644 --- a/frontend/src/features/questionFeed/QuestionFeed.tsx +++ b/frontend/src/features/questionFeed/QuestionFeed.tsx @@ -28,8 +28,11 @@ * volume-rewarding/direction-neutral, the ONLY reward surface; no streak/score/confetti) * and the quiet "confirm-lands" fade (ANNEX C) shown on a successful confirm/pick while the * next item is in flight. - * - Preserved verbatim: Level 1/2/3 flows, `getAutoTagChips` auto-tagging on candidate pick, - * no-re-presentation (`rejectedCandidateIds`), the singleton-NO terminal vote, per-item + * - Preserved verbatim: the candidate question's interaction contract (issue #728 removed + * the level1 -> level2 funnel, not the answers - see the de-laddering notes at + * `initialStage`'s old site, `rejectSuggestion`, and `candidateQuestionBody`), + * `getAutoTagChips` auto-tagging on candidate pick, no-re-presentation + * (`rejectedCandidateIds`), the singleton-NO terminal vote, per-item * state reset inside the fetch `.then()` (not a keyed `useEffect` - the stale-filter fix), * the rate-limit banner, `data-card-*` attributes + the `mpc:card-selected` event (via * `getPrintingCandidateDataAttributes`, unchanged), every `data-testid` this file's own @@ -82,7 +85,6 @@ import { MysteryCard, randomFlavorText, RevealWrapper, - StaticCardPanel, ZoomableThumbnail, } from "@/features/printingTags/cardPanel"; import { WhatsThatWords } from "@/features/questionFeed/WhatsThatWords"; @@ -98,7 +100,6 @@ import { selectRemoteBackendURL } from "@/store/slices/backendSlice"; import { setNotification } from "@/store/slices/toastsSlice"; type FollowUp = "none" | "no-match-reason"; -type CandidateStage = "level1" | "level2" | "level3"; // --------------------------------------------------------------------------------------- // Layout primitives (SPEC-wtc-rebuild.md section 1c's per-element binding table + section 3's @@ -695,12 +696,6 @@ function normalizeQuestionFeedCounts( return raw; } -function initialStage(item: QuestionFeedItem | null): CandidateStage { - return item?.type === "confirm_suggestion" && item?.suggestedPrinting != null - ? "level1" - : "level2"; -} - export function QuestionFeed() { const dispatch = useAppDispatch(); const backendURL = useAppSelector(selectRemoteBackendURL); @@ -736,11 +731,11 @@ export function QuestionFeed() { initialChipStates() ); const [followUp, setFollowUp] = useState("none"); - // Candidate identifiers the user has explicitly said NO to (Level 1 only - "Not sure" is - // genuine uncertainty, not a rejection, and deliberately never adds here) within THIS item's - // flow - reset on every new item below. Design rule (owner-directed): a candidate the user - // has just rejected is never re-presented as a selectable answer at a later level within the - // same item - see rejectSuggestion below and the filtered candidate list this feeds. + // The one candidate the user has explicitly said NO to within THIS item's flow (issue #728: + // with the ladder gone this can only ever be the suggested candidate - the only candidate + // rejected by name; "Not sure" is genuine uncertainty, not a rejection, and deliberately + // never adds here). Drives the suggestion slot's "you said not this one" collapse - reset on + // every new item below. const [rejectedCandidateIds, setRejectedCandidateIds] = useState>( new Set() ); @@ -758,10 +753,14 @@ export function QuestionFeed() { // gets a persistent inline notice rather than a transient, alarm-toned toast. const [rateLimited, setRateLimited] = useState(false); - const [stage, setStage] = useState("level2"); - // Collapsed by default (decision: chip-as-filter survives on Level 2, but off-path for the - // common case). Selecting a candidate below ignores this entirely; it only ever narrows - // which tiles are shown. + // The de-laddered feed (issue #728) has no fixed level1 -> level2 -> level3 sequence. The + // only remaining "stage" is the data-driven post-selection attribute confirmation (formerly + // Level 3), entered solely when a selected candidate leaves an exclusion group open - never + // a fixed-sequence step. + const [level3Active, setLevel3Active] = useState(false); + // Collapsed by default (decision: chip-as-filter survives on the candidate grid, but + // off-path for the common case). Selecting a candidate below ignores this entirely; it only + // ever narrows which tiles are shown. const [filterExpanded, setFilterExpanded] = useState(false); // Level 3 only ever asks about groups an already-selected candidate left open - keyed by // tagName, but only ever contains chips from getOpenExclusionGroups(pendingCandidate). @@ -830,9 +829,9 @@ export function QuestionFeed() { setConfirmedArtistName(null); setRateLimited(false); // Issue #707 / A4 amendment - shown automatically for the two candidate-type shapes - // (identify_printing's shortlist, confirm_suggestion once dropped to Level 2), where - // the attribute chips actually narrow something; artist/tag items never reach the - // branch that reads this at all, so their default is moot. + // (identify_printing's shortlist, confirm_suggestion's candidate page), where the + // attribute chips actually narrow something; artist/tag items never reach the branch + // that reads this at all, so their default is moot. setFilterExpanded( newItem != null && (newItem.type === "identify_printing" || @@ -840,7 +839,9 @@ export function QuestionFeed() { ); setLevel3ChipStates({}); setLanded(false); - setStage(initialStage(newItem)); + // The only remaining "stage" (issue #728) is the post-selection attribute + // confirmation - every new item starts outside it. + setLevel3Active(false); }) .catch(() => { setItem(null); @@ -957,7 +958,7 @@ export function QuestionFeed() { ) ) ); - setStage("level3"); + setLevel3Active(true); } else { setLanded(true); advance(); @@ -1106,7 +1107,9 @@ export function QuestionFeed() { // Records the "Not sure" abstention (issue #712) and moves on - fire-and-forget, same // best-effort convention as the auto-tag-chip casts in selectCandidate above: the write is - // informative, not gating, so a failed request never blocks the stage transition. + // informative, not gating, so a failed request never blocks the transition. There is no + // ladder to fall into (issue #728): "Not sure" means "I can't resolve this", so it advances + // to the next question rather than re-asking the same candidates on another page. const submitNotSure = () => { if (backendURL != null && item != null) { APISubmitQuestionAbstention( @@ -1116,14 +1119,15 @@ export function QuestionFeed() { item.type ).catch(() => undefined); } - setStage("level2"); + advance(); }; - // Level 1's NO. In the general case this casts no vote itself - there's no backend concept of - // "reject just this one candidate specifically," only a positive vote for a specific printing - // or a generic isNoMatch for the whole set (see selectCandidate above) - so it purely records - // the rejection client-side so Level 2's candidate list (below) excludes it, then falls - // through to the SAME setStage("level2") transition as before. + // The suggestion slot's NO. In the general case this casts no vote itself - there's no + // backend concept of "reject just this one candidate specifically," only a positive vote for + // a specific printing or a generic isNoMatch for the whole set (see selectCandidate above) - + // so it purely records the rejection client-side and collapses the suggestion slot into the + // "you said not this one" context; the remaining candidate grid (below) carries the question + // on the SAME page. No stage transition (issue #728). // // EXCEPTION - the singleton case (owner-reported dedup bug, docs/features/printing-tags.md's // questionFeed section): when the suggested printing is the card's ONLY candidate, rejecting @@ -1132,14 +1136,12 @@ export function QuestionFeed() { // that gap: the vote persists at the moment "No" is tapped, with or without any further tap. const rejectSuggestion = () => { if (item?.suggestedPrinting == null) { - setStage("level2"); return; } const rejectedIdentifier = item.suggestedPrinting.identifier; setRejectedCandidateIds((previous) => new Set(previous).add(rejectedIdentifier) ); - setStage("level2"); const remainingCandidates = (item.candidates ?? []).filter( (candidate) => candidate.identifier !== rejectedIdentifier ); @@ -1194,19 +1196,27 @@ export function QuestionFeed() { const isCandidateType = item.type === "confirm_suggestion" || item.type === "identify_printing"; const allCandidates = item.candidates ?? []; - // Excludes anything the user rejected at Level 1 ("No" - see rejectSuggestion) BEFORE the - // chip filter applies, so a rejected candidate is never offered again as a selectable tile - // for the rest of this item's flow, regardless of chip state. - const nonRejectedCandidates = allCandidates.filter( - (candidate) => !rejectedCandidateIds.has(candidate.identifier) + // Issue #728 - the suggested candidate is judged exactly ONCE, in its own slot above, and is + // never re-offered as a grid tile (the old Level 2 re-presented it "highlighted" - the same + // candidate asked about twice). The grid is the REST of the candidates; the rejected set + // (which only ever holds the suggested id, see rejectSuggestion) is belt-and-braces for that + // same exclusion and drives the "you said not this one" context above. + const suggestedCandidateId = + item.type === "confirm_suggestion" + ? item.suggestedPrinting?.identifier ?? null + : null; + const gridCandidates = allCandidates.filter( + (candidate) => + candidate.identifier !== suggestedCandidateId && + !rejectedCandidateIds.has(candidate.identifier) ); const visibleCandidates = filterCandidatesByChipStates( - nonRejectedCandidates, + gridCandidates, chipStates ); - const hiddenCount = nonRejectedCandidates.length - visibleCandidates.length; + const hiddenCount = gridCandidates.length - visibleCandidates.length; - // Issue #503 (WTC phase C1) - group the level-2 grid by shared Scryfall illustration. A + // Issue #503 (WTC phase C1) - group the candidate grid by shared Scryfall illustration. A // cluster only forms for >=2 candidates sharing a non-null illustrationId; every other // candidate - a unique illustrationId, or no illustrationId at all // (CanonicalPrintingMetadata.illustration_id is nullable and frequently absent, see @@ -1242,18 +1252,24 @@ export function QuestionFeed() { (candidate) => !groupedCandidateIds.has(candidate.identifier) ); - const suggestionRejectedWithNoneLeft = + // The user tapped "No, different printing" - the suggestion slot collapses into a "you said + // not this one" context line and the remaining grid carries the question (issue #728: this + // stays on the same page; it is not a stage transition). + const suggestionRejected = item.type === "confirm_suggestion" && item.suggestedPrinting != null && - rejectedCandidateIds.has(item.suggestedPrinting.identifier) && - nonRejectedCandidates.length === 0; + rejectedCandidateIds.has(item.suggestedPrinting.identifier); + // Singleton rejection (owner-reported dedup bug, docs/features/printing-tags.md): when the + // suggested printing was the card's ONLY candidate, rejecting it empties the grid and + // rejectSuggestion already cast the "None of these" vote - this only gates presentation. + const suggestionRejectedWithNoneLeft = + suggestionRejected && gridCandidates.length === 0; // Shape (d) - open-ended (ANNEX B): an `identify_printing` item with no shortlist at all // (the smallest slice - cold-start/no-evidence). Framed as the "tricky one" (WD7) instead of // the neutral pick-grid shape. const isOpenEndedShape = isCandidateType && - stage === "level2" && item.type === "identify_printing" && allCandidates.length === 0; @@ -1319,119 +1335,19 @@ export function QuestionFeed() { ); - // The one card panel Level 2 ever renders now (issue #707) - the attribute-chip panel no - // longer replaces this with a ring-around-card composition; it renders separately in QPanel - // (level2Body below) instead, so the pinned reference card (Subject, A2) is never swapped - // out or occluded by it. + // The one card panel the candidate question renders (issue #707) - the attribute-chip panel + // no longer replaces this with a ring-around-card composition; it renders separately in + // QPanel (candidateQuestionBody below) instead, so the pinned reference card (Subject, A2) + // is never swapped out or occluded by it. const plainCardPanel = ( {subjectCard} ); - // Level 1 only - the compact single-card confirmation screen. - const level1CardPanel = ( - - {subjectCard} - - ); - let cardNode: React.ReactNode; let questionsNode: React.ReactNode; if (isCandidateType) { - if (stage === "level1" && item.suggestedPrinting != null) { - cardNode = level1CardPanel; - questionsNode = ( -
- {!revealed ? ( -
- -
- ) : ( - <> - - - Suggested match - - - - - - - - {`${item.suggestedPrinting.expansionCode} - - - - - {item.card.name} - - {" "} - {item.suggestedPrinting.expansionCode.toUpperCase()}{" "} - {item.suggestedPrinting.collectorNumber} - - - - Is it this one? - - - - - - item.suggestedPrinting != null && - selectCandidate(item.suggestedPrinting, false) - } - data-testid="question-feed-level1-yes" - > - {submitting ? : "Yes — that's the one"} - - - - Not sure - - - No, different printing - - - Skip - - - - {landed && ( - - ✓ Tagged — nice. Next card loading… - - )} - - )} -
- ); - } else if (stage === "level3") { + if (level3Active) { cardNode = plainCardPanel; questionsNode = (
@@ -1489,8 +1405,10 @@ export function QuestionFeed() {
); } else { - // Level 2 - the candidate grid, or (isOpenEndedShape) the dashed "tricky one" framing - // for a zero-candidate identify_printing item (shape d, ANNEX B). + // The single candidate question (issue #728 - the level1/level2 ladder is gone): the + // candidate grid, or (isOpenEndedShape) the dashed "tricky one" framing for a + // zero-candidate identify_printing item (shape d, ANNEX B). The suggested candidate + // (when present) renders in its own slot above the grid and never as a tile. cardNode = plainCardPanel; const shapePillClass = item.type === "confirm_suggestion" @@ -1525,12 +1443,6 @@ export function QuestionFeed() { ) => ( { @@ -1576,14 +1488,22 @@ export function QuestionFeed() { NO_MATCH_REASON_TAG_GROUPS["not-official-printing"] .tagNames as readonly string[] ).includes(chosenTagName); - if (isNotOfficialPrinting && nonRejectedCandidates.length > 0) { + if (isNotOfficialPrinting && gridCandidates.length > 0) { setFollowUp("none"); setFilterExpanded(true); return; } advance(); }; - const level2Body = ( + // The de-laddered candidate question (issue #728): the suggested candidate (when + // present) is asked about ONCE, in its own slot, with the rest of the candidates in the + // grid on the SAME page - no level1 -> level2 funnel re-presents it as a highlighted + // tile. "Art matches, not an official printing" is a first-class fallback here (bottom + // row), not a Level-2-only escape hatch. Answers resolve or advance by what they mean: + // Yes casts the vote; No collapses this slot and leaves the grid to carry the question; + // Not sure records an abstention and advances; the bottom row resolves the whole + // question (none of these / custom art / skip). + const candidateQuestionBody = ( <> {item.type === "confirm_suggestion" && item.suggestedPrinting != null && - (suggestionRejectedWithNoneLeft ? ( + !suggestionRejected && ( + <> + + + + + + {`${item.suggestedPrinting.expansionCode} + + + + + {item.card.name} + + {" "} + {item.suggestedPrinting.expansionCode.toUpperCase()}{" "} + {item.suggestedPrinting.collectorNumber} + + + + Is it this one? + + + + + + item.suggestedPrinting != null && + selectCandidate(item.suggestedPrinting, false) + } + data-testid="question-feed-suggestion-yes" + > + {submitting ? : "Yes — that's the one"} + + + + Not sure + + + No, different printing + + + + {landed && ( + + ✓ Tagged — nice. Next card loading… + + )} + + )} + {item.type === "confirm_suggestion" && + item.suggestedPrinting != null && + suggestionRejected && ( <> Got it - not that one. Is it any official printing at all? @@ -1623,14 +1612,7 @@ export function QuestionFeed() { - ) : ( - - Which of these is it?{" "} - {" "} - {item.suggestedPrinting.expansionCode.toUpperCase()}{" "} - {item.suggestedPrinting.collectorNumber} was suggested - - ))} + )} {item.type === "identify_printing" && ( {isOpenEndedShape @@ -1810,11 +1792,13 @@ export function QuestionFeed() { ) : isOpenEndedShape ? ( -
- {level2Body} +
+ {candidateQuestionBody}
) : ( -
{level2Body}
+
+ {candidateQuestionBody} +
); } } else { diff --git a/frontend/tests/CardbackPdfWaitFidelity.spec.ts b/frontend/tests/CardbackPdfWaitFidelity.spec.ts index ef61933fd..29d3af009 100644 --- a/frontend/tests/CardbackPdfWaitFidelity.spec.ts +++ b/frontend/tests/CardbackPdfWaitFidelity.spec.ts @@ -239,9 +239,9 @@ for (const viewport of [ "rgb(11, 21, 32)" ); - // E.4 `.tbtn.yes` (ThumbButton, QuestionFeed's own shipped idiom, reproduced verbatim) - - // min-height 44px floor. - const yesButton = embed.getByTestId("question-feed-level1-yes"); + // E.4 `.tbtn.yes` equivalent (the suggestion slot's primary Yes button, QuestionFeed's + // own shipped idiom, reproduced verbatim) - min-height 44px floor. + const yesButton = embed.getByTestId("question-feed-suggestion-yes"); await expect(yesButton).toBeVisible({ timeout: 15_000 }); const yesHeight = await yesButton.evaluate( (el) => el.getBoundingClientRect().height diff --git a/frontend/tests/PDFWaitExperience.spec.ts b/frontend/tests/PDFWaitExperience.spec.ts index 899a7b9f9..c7d88980a 100644 --- a/frontend/tests/PDFWaitExperience.spec.ts +++ b/frontend/tests/PDFWaitExperience.spec.ts @@ -138,11 +138,18 @@ test.describe("PDF-generation wait experience (SPEC-cardback-pdfwait.md §D, PKG const embed = page.getByTestId("pdf-wait-game"); await expect(embed).toBeVisible({ timeout: 15_000 }); - // The real, unforked QuestionFeed funnel - Level 1 YES/NOT SURE/NO/SKIP. + // The real, unforked QuestionFeed - the de-laddered single-page candidate question + // (issue #728): the suggested-match slot (YES/NOT SURE/NO) with the rest of the + // candidates on the same page, no level1 -> level2 funnel. await expect(embed.getByTestId("question-feed")).toBeVisible({ timeout: 15_000, }); - await expect(embed.getByTestId("question-feed-level1")).toBeVisible(); + await expect( + embed.getByTestId("question-feed-candidate-question") + ).toBeVisible(); + await expect( + embed.getByTestId("question-feed-suggestion-yes") + ).toBeVisible(); await expect(embed.getByTestId("pdf-wait-game-ribbon")).toBeVisible(); })(), ]); diff --git a/frontend/tests/QuestionFeed.spec.ts b/frontend/tests/QuestionFeed.spec.ts index 038cb6540..cc4ca4549 100644 --- a/frontend/tests/QuestionFeed.spec.ts +++ b/frontend/tests/QuestionFeed.spec.ts @@ -530,7 +530,7 @@ test.describe("question feed - Level 2 illustration grouping", () => { }); test.describe("question feed - confirm_suggestion question type", () => { - test("lands on Level 1 - a single suggested printing, no grid - and shows the 'Is it this one?' prompt", async ({ + test("lands on the suggested-match question - the suggestion is asked about in its own slot on page one, never re-judged as a grid tile", async ({ page, network, }) => { @@ -540,21 +540,28 @@ test.describe("question feed - confirm_suggestion question type", () => { await expect( page.getByTestId("question-feed-suggestion-prompt") ).toContainText("Is it this one?"); - await expect(page.getByTestId("question-feed-level1-yes")).toBeVisible(); - // no candidate grid at Level 1 - only reachable via NOT SURE/NO + await expect( + page.getByTestId("question-feed-suggestion-yes") + ).toBeVisible(); + // Issue #728 - the de-laddered feed shows the rest of the candidates on the SAME page, + // and the suggested candidate is never re-presented as a tile (judged once, in its slot). await expect( page.locator(`[data-card-identifier="${printingCandidate1.identifier}"]`) ).toHaveCount(0); + await expect( + page.locator(`[data-card-identifier="${printingCandidate2.identifier}"]`) + ).toBeVisible(); - // Regression check (#49 dropped this): Level 1 still needs its own reference render of the - // suggested printing to compare against - "Is it this one?" is unanswerable from text alone. - // `getByRole("img")` (not a plain `img` locator) - round 3's shared `` (own - // comment, cardPanel.tsx) renders a SECOND `` in this same container (its own "?" glyph, - // `alt=""`), which a bare `locator("img")` now matches too, causing a Playwright strict-mode - // violation. `alt=""` strips an from the accessibility tree entirely, so `getByRole` - // (unlike a tag-selector) unambiguously resolves to just the real reference thumbnail below. + // Regression check (#49 dropped this): the suggestion slot still needs its own reference + // render of the suggested printing to compare against - "Is it this one?" is + // unanswerable from text alone. `getByRole("img")` (not a plain `img` locator) - round + // 3's shared `` (own comment, cardPanel.tsx) renders a SECOND `` in + // this same container (its own "?" glyph, `alt=""`), which a bare `locator("img")` now + // matches too, causing a Playwright strict-mode violation. `alt=""` strips an from + // the accessibility tree entirely, so `getByRole` (unlike a tag-selector) unambiguously + // resolves to just the real reference thumbnail below. const referenceImage = page - .getByTestId("question-feed-level1-reference-image") + .getByTestId("question-feed-suggestion-reference-image") .getByRole("img"); await expect(referenceImage).toBeVisible(); await expect(referenceImage).toHaveAttribute( @@ -563,7 +570,7 @@ test.describe("question feed - confirm_suggestion question type", () => { ); }); - test("YES confirms the suggested printing directly, without visiting the grid", async ({ + test("YES on the suggestion casts the printing vote for it directly", async ({ page, network, }) => { @@ -581,19 +588,20 @@ test.describe("question feed - confirm_suggestion question type", () => { }); await loadPageWithDefaultBackend(page, "whatsthat"); - await page.getByTestId("question-feed-level1-yes").click(); + await page.getByTestId("question-feed-suggestion-yes").click(); await expect .poll(() => submittedPrintingIdentifier) .toBe(printingCandidate1.identifier); }); - test("NOT SURE drops to Level 2's candidate grid without casting a printing vote, but does POST an abstention", async ({ + test("NOT SURE records an abstention and advances to the next question, without casting a printing vote", async ({ page, network, }) => { let printingTagSubmitted = false; let abstentionBody: { identifier?: string; questionType?: string } = {}; + let feedFetchCount = 0; network.use( questionFeedConfirmSuggestion, submitQuestionAbstentionRecorded, @@ -606,32 +614,31 @@ test.describe("question feed - confirm_suggestion question type", () => { if (request.url().includes("/2/submitQuestionAbstention/")) { abstentionBody = request.postDataJSON(); } + if (request.url().includes("/2/questionFeed/")) { + feedFetchCount += 1; + } }); await loadPageWithDefaultBackend(page, "whatsthat"); - await page.getByTestId("question-feed-level1-not-sure").click(); + await page.getByTestId("question-feed-suggestion-not-sure").click(); - const suggestedCandidate = page.locator( - `[data-card-identifier="${printingCandidate1.identifier}"]` - ); - await expect(suggestedCandidate).toBeVisible(); - await expect(suggestedCandidate).toHaveClass(/highlighted/); + // Issue #728 - "Not sure" means "I can't resolve this": abstention + advance to the next + // question, not a second page re-judging the same candidates. expect(printingTagSubmitted).toBe(false); - + await expect.poll(() => feedFetchCount).toBeGreaterThanOrEqual(2); await expect .poll(() => abstentionBody.identifier) .toBe(cardDocument1.identifier); expect(abstentionBody.questionType).toBe("confirm_suggestion"); }); - test("NO drops to Level 2's candidate grid, excluding the rejected suggestion, without casting a vote", async ({ + test("NO on the suggestion collapses its slot (never a selectable tile again) and keeps the remaining candidates selectable on the same page, without casting a vote", async ({ page, network, }) => { - // Double-asking fix: a candidate the user just rejected at Level 1 is never - // re-presented as a selectable tile at Level 2 within the same item - see - // rejectSuggestion/rejectedCandidateIds in QuestionFeed.tsx. This mock has two - // candidates, so the remaining one (printingCandidate2) should still show. + // Issue #728 - the rejected suggestion is never re-presented as a selectable tile; the + // remaining candidate (printingCandidate2) stays selectable on the SAME page (no stage + // transition). See rejectSuggestion/rejectedCandidateIds in QuestionFeed.tsx. let printingTagSubmitted = false; network.use(questionFeedConfirmSuggestion, ...defaultHandlers); page.on("request", (request) => { @@ -641,7 +648,7 @@ test.describe("question feed - confirm_suggestion question type", () => { }); await loadPageWithDefaultBackend(page, "whatsthat"); - await page.getByTestId("question-feed-level1-no").click(); + await page.getByTestId("question-feed-suggestion-no").click(); await expect( page.locator(`[data-card-identifier="${printingCandidate1.identifier}"]`) @@ -649,6 +656,10 @@ test.describe("question feed - confirm_suggestion question type", () => { await expect( page.locator(`[data-card-identifier="${printingCandidate2.identifier}"]`) ).toBeVisible(); + // contextual copy replaces the suggestion slot's "Is it this one?" + await expect( + page.getByTestId("question-feed-suggestion-prompt") + ).toContainText("Is it any official printing at all?"); expect(printingTagSubmitted).toBe(false); }); @@ -656,16 +667,17 @@ test.describe("question feed - confirm_suggestion question type", () => { page, network, }) => { - // Owner-reported dedup bug (docs/features/printing-tags.md's questionFeed section): Level 1 - // "Is it M21 203?" -> NO, where M21 203 was the card's ONLY candidate. Previously (this test - // used to assert `printingTagSubmitted === false` here - that assertion WAS the bug, not a - // correct behavior spec) "No" cast no vote at all and merely revealed a further "None of - // these" tap the user still had to make; if that tap never happened, no CardPrintingTag row - // ever existed for question_feed.py's tier-1 exclusion to match against, so the exact same - // question resurfaced on the next feed fetch. Since there is nothing else this card's "No" - // could mean (no other candidate exists), it must now be treated as the terminal answer: - // the same isNoMatch vote "None of these" itself casts is submitted the moment "No" is - // tapped, with no further tap required. + // Owner-reported dedup bug (docs/features/printing-tags.md's questionFeed section): the + // suggestion slot's "Is it M21 203?" -> NO, where M21 203 was the card's ONLY candidate. + // Previously (this test used to assert `printingTagSubmitted === false` here - that + // assertion WAS the bug, not a correct behavior spec) "No" cast no vote at all and merely + // revealed a further "None of these" tap the user still had to make; if that tap never + // happened, no CardPrintingTag row ever existed for question_feed.py's tier-1 exclusion to + // match against, so the exact same question resurfaced on the next feed fetch. Since there + // is nothing else this card's "No" could mean (no other candidate exists), it must now be + // treated as the terminal answer: the same isNoMatch vote "None of these" itself casts is + // submitted the moment "No" is tapped, with no further tap required. (Issue #728 - the + // ladder is gone, so this lives on the same page; the singleton path is unchanged.) let submittedPrinting: { printingIdentifier?: string; isNoMatch?: boolean; @@ -682,7 +694,7 @@ test.describe("question feed - confirm_suggestion question type", () => { }); await loadPageWithDefaultBackend(page, "whatsthat"); - await page.getByTestId("question-feed-level1-no").click(); + await page.getByTestId("question-feed-suggestion-no").click(); // the rejected candidate is never a selectable tile again await expect( @@ -709,11 +721,12 @@ test.describe("question feed - confirm_suggestion question type", () => { network, }) => { // Regression guard for a real-device-only bug (not reproducible in this sandbox's - // Chromium): Level 1 previously reused Level 2's sticky, negative-z-index CardPanel for - // its own short single-screen layout, which composited incorrectly on a real phone - - // answer controls painted overlapping the card art instead of cleanly below it. The fix - // (StaticCardPanel - see cardPanel.tsx) puts everything back in normal document flow; - // this asserts that property directly via bounding-box math rather than relying on visual + // Chromium): the pinned reference card (Subject, A2) used a sticky, negative-z-index + // CardPanel that composited incorrectly on a real phone at narrow widths - answer controls + // painted overlapping the card art instead of cleanly below it. The de-laddered feed + // (issue #728) renders this same card panel for every candidate question, so the guard + // now covers the whole single-page surface: suggestion slot + bottom row. This asserts + // the non-overlap property directly via bounding-box math rather than relying on visual // diffing this sandbox can't validate against real hardware anyway. network.use(questionFeedConfirmSuggestion, ...defaultHandlers); await page.setViewportSize({ width: 390, height: 844 }); @@ -721,9 +734,11 @@ test.describe("question feed - confirm_suggestion question type", () => { // The card's full box (art + starburst + name caption), not just the - the // real-device bug this guards against overlapped the caption too, not only the artwork. - const cardPanel = page.getByTestId("question-feed-level1-card-panel"); + const cardPanel = page.getByTestId("question-feed-card-panel"); await expect(page.getByAltText(cardDocument1.name)).toBeVisible(); - await expect(page.getByTestId("question-feed-level1-yes")).toBeVisible(); + await expect( + page.getByTestId("question-feed-suggestion-yes") + ).toBeVisible(); const cardBox = await cardPanel.boundingBox(); expect(cardBox).not.toBeNull(); @@ -731,10 +746,12 @@ test.describe("question feed - confirm_suggestion question type", () => { const controls = [ page.getByTestId("question-feed-tier-badge"), page.getByTestId("question-feed-suggestion-prompt"), - page.getByTestId("question-feed-level1-yes"), - page.getByTestId("question-feed-level1-not-sure"), - page.getByTestId("question-feed-level1-no"), - page.getByTestId("question-feed-level1-skip"), + page.getByTestId("question-feed-suggestion-yes"), + page.getByTestId("question-feed-suggestion-not-sure"), + page.getByTestId("question-feed-suggestion-no"), + page.getByTestId("question-feed-no-match"), + page.getByTestId("question-feed-custom-art"), + page.getByTestId("question-feed-skip"), ]; for (const control of controls) { const controlBox = await control.boundingBox(); diff --git a/frontend/tests/QuestionFeedResponsive.spec.ts b/frontend/tests/QuestionFeedResponsive.spec.ts index 8ed5b8213..8d9a0a80b 100644 --- a/frontend/tests/QuestionFeedResponsive.spec.ts +++ b/frontend/tests/QuestionFeedResponsive.spec.ts @@ -298,7 +298,7 @@ test.describe("question feed - container-first hero layout (section 3, WTC rebui // `.btn { min-height: 44px }` base class (SPEC-wtc-rebuild.md section 1c) instead of the // retired `ThumbButton`/`FilterToggleButton` styled overrides. test.describe("question feed - tap target sizes (mobile funnel pass)", () => { - test("Level 1's stacked answer buttons meet the 44px floor", async ({ + test("the suggested-match slot's answer buttons meet the 44px floor", async ({ page, network, }) => { @@ -307,10 +307,9 @@ test.describe("question feed - tap target sizes (mobile funnel pass)", () => { await loadPageWithDefaultBackend(page, "whatsthat"); for (const testId of [ - "question-feed-level1-yes", - "question-feed-level1-not-sure", - "question-feed-level1-no", - "question-feed-level1-skip", + "question-feed-suggestion-yes", + "question-feed-suggestion-not-sure", + "question-feed-suggestion-no", ]) { const box = await page.getByTestId(testId).boundingBox(); expect(box).not.toBeNull(); @@ -687,14 +686,14 @@ test.describe("question feed - desktop uses its available horizontal space (issu // state as before but stops clipping for exactly the hover duration ZoomableThumbnail's own // hover rule (cardPanel.tsx) scales the art up, so the zoom is no longer cut flush at the edge. test.describe("question feed - hover-zoom is not clipped by its frame (issue #705)", () => { - test("the Level 1 reference thumbnail's frame stops clipping while hovered", async ({ + test("the suggestion slot's reference thumbnail frame stops clipping while hovered", async ({ page, network, }) => { network.use(questionFeedConfirmSuggestion, ...defaultHandlers); await loadPageWithDefaultBackend(page, "whatsthat"); - const thumb = page.getByTestId("question-feed-level1-reference-image"); + const thumb = page.getByTestId("question-feed-suggestion-reference-image"); await expect(thumb).toBeVisible(); const restOverflow = await thumb.evaluate( (el) => window.getComputedStyle(el).overflow @@ -709,11 +708,11 @@ test.describe("question feed - hover-zoom is not clipped by its frame (issue #70 }); }); -// Issue #711 / #740 - the Level 1 Yes button carries neither the oversized `.big` modifier -// nor the full-width `.block` modifier, so it reads at the same font size and a +// Issue #711 / #740 - the suggestion slot's Yes button carries neither the oversized `.big` +// modifier nor the full-width `.block` modifier, so it reads at the same font size and a // content-sized width, like its ActionGrid siblings. Hierarchy comes from position (its own // row, above the grid) and its `.primary` colour, not from disproportionate size. -test.describe("question feed - Level 1 answer-row hierarchy (issue #711)", () => { +test.describe("question feed - suggestion-slot answer-row hierarchy (issue #711)", () => { test("the Yes button reads at the same font size as its 'Not sure' sibling", async ({ page, network, @@ -721,8 +720,8 @@ test.describe("question feed - Level 1 answer-row hierarchy (issue #711)", () => network.use(questionFeedConfirmSuggestion, ...defaultHandlers); await loadPageWithDefaultBackend(page, "whatsthat"); - const yesButton = page.getByTestId("question-feed-level1-yes"); - const notSureButton = page.getByTestId("question-feed-level1-not-sure"); + const yesButton = page.getByTestId("question-feed-suggestion-yes"); + const notSureButton = page.getByTestId("question-feed-suggestion-not-sure"); await expect(yesButton).toBeVisible(); await expect(notSureButton).toBeVisible(); @@ -743,8 +742,8 @@ test.describe("question feed - Level 1 answer-row hierarchy (issue #711)", () => await page.setViewportSize({ width: 800, height: 900 }); await loadPageWithDefaultBackend(page, "whatsthat"); - const yesButton = page.getByTestId("question-feed-level1-yes"); - const notSureButton = page.getByTestId("question-feed-level1-not-sure"); + const yesButton = page.getByTestId("question-feed-suggestion-yes"); + const notSureButton = page.getByTestId("question-feed-suggestion-not-sure"); await expect(yesButton).toBeVisible(); await expect(notSureButton).toBeVisible(); From e58264047ff794d5d034e5e4208d69ba6f3f57ae Mon Sep 17 00:00:00 2001 From: wilfordgrimley <2397930+WilfordGrimley@users.noreply.github.com> Date: Sat, 8 Aug 2026 23:11:21 +0000 Subject: [PATCH 2/2] test(question-feed): ack removed fixed-ladder spec titles in coverage gate --- .github/coverage-acks.txt | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/.github/coverage-acks.txt b/.github/coverage-acks.txt index 6c3f0028d..08111fa9b 100644 --- a/.github/coverage-acks.txt +++ b/.github/coverage-acks.txt @@ -161,3 +161,11 @@ coverage-ack: frontend/tests/QuestionFeed.spec.ts::question feed - Level 2 (cand # the same underlying hard constraint (chips must never occlude the reference card) against the # new mechanism instead of the retired one. coverage-ack: frontend/tests/QuestionFeedResponsive.spec.ts::question feed - Level 2 layout containment (real-device regression guard) > at 360px with the attribute-chip filter expanded, the ring collapses to a stack instead of squeezing the card — dropped: the ring-around-card composition (CardArea) it guarded is retired by issue #707 (chips render in QPanel now, never sharing a box with the card); replaced by "at 360px with the attribute-chip filter shown, the chips never overlap the pinned reference card" covering the same never-occlude constraint against the new mechanism. +coverage-ack: frontend/tests/QuestionFeedResponsive.spec.ts::question feed - tap target sizes (mobile funnel pass) > Level 1's stacked answer buttons meet the 44px floor — the fixed Level 1/2/3 ladder was removed by the #728 de-hardcoding; this flow no longer exists +coverage-ack: frontend/tests/QuestionFeedResponsive.spec.ts::question feed - hover-zoom is not clipped by its frame (issue #705) > the Level 1 reference thumbnail's frame stops clipping while hovered — the fixed Level 1/2/3 ladder was removed by the #728 de-hardcoding; this flow no longer exists +coverage-ack: frontend/tests/QuestionFeedResponsive.spec.ts::question feed - Level 1 answer-row hierarchy (issue #711) > the Yes button sizes to its content instead of spanning full width (issue #740) — the fixed Level 1/2/3 ladder was removed by the #728 de-hardcoding; this flow no longer exists +coverage-ack: frontend/tests/QuestionFeedResponsive.spec.ts::question feed - Level 1 answer-row hierarchy (issue #711) > the Yes button reads at the same font size as its 'Not sure' sibling — the fixed Level 1/2/3 ladder was removed by the #728 de-hardcoding; this flow no longer exists +coverage-ack: frontend/tests/QuestionFeed.spec.ts::question feed - confirm_suggestion question type > lands on Level 1 - a single suggested printing, no grid - and shows the 'Is it this one?' prompt — the fixed Level 1/2/3 ladder was removed by the #728 de-hardcoding; suggestion and grid now render on a single page +coverage-ack: frontend/tests/QuestionFeed.spec.ts::question feed - confirm_suggestion question type > YES confirms the suggested printing directly, without visiting the grid — the fixed Level 1/2/3 ladder was removed by the #728 de-hardcoding; the grid now coexists with the suggestion on the same page +coverage-ack: frontend/tests/QuestionFeed.spec.ts::question feed - confirm_suggestion question type > NOT SURE drops to Level 2's candidate grid without casting a printing vote, but does POST an abstention — the fixed Level 1/2/3 ladder was removed by the #728 de-hardcoding; the grid now coexists with the suggestion on the same page +coverage-ack: frontend/tests/QuestionFeed.spec.ts::question feed - confirm_suggestion question type > NO drops to Level 2's candidate grid, excluding the rejected suggestion, without casting a vote — the fixed Level 1/2/3 ladder was removed by the #728 de-hardcoding; the grid now coexists with the suggestion on the same page