fix(android): pick submit leaf only from the bottom-most row, not the whole panel (#335)

Reviewer cross-check of 2ce59eb on real dumps found a Samsung sample where the true bottom bar is a label-less 31px FrameLayout whose only text node is zero-size ([0,0][0,0]). A whole-panel leaf scan fell through to the next lowest labelled leaf, which lives in the PAYMENT-METHOD row directly above the bottom bar ("使用#微信支付，更换先用后付可0元下单"), and the climb-to-clickable-ancestor landed the click on "change payment method" instead of the order button. That must never happen. finalSubmitTargets is now a strict two-step "row, then leaf" pick instead of a single whole-panel leaf scan: 1. Row: among visible/enabled/non-zero-size CLICKABLE nodes inside the recognized panel's container with height <= 30% of screen height (the guard from 2bc624f), pick the one with the lowest bottom edge; ties go to the rightmost. This row, and only this row, may hold the order button. If its own subtree matches a payment-method alias (textAliases.specPanel.paymentAreaAliases: 微信支付/先用后付/支付方式) or SUBMIT_TARGET_BLOCKED_MARKERS, fail explicitly (outcome=row_blocked) - never fall back to a higher row. 2. Leaf: within that row's own subtree (or the row itself), among visible/non-zero-size nodes with a non-blank own label whose nearest clickable ancestor is EXACTLY that row, pick the bottom-right-most one (unchanged rationale from 2ce59eb: PurchaseUiDriver.clickFresh re-finds by preferredOrDescendantLabel()+className+center±32 before climbing to the nearest clickable ancestor, so the target must carry a real label). If the row has no such leaf, fail explicitly (outcome=bottom_row_unlabelled) - never fall back to a higher row either. hasFinalSavedAddressEvidence, finalConfirmation and submitOrderOnce still all consume this one finalSubmitTargets. Tests: SpecPanelFixtures.sheet(Sheet(submit = "hidden")) already models the Samsung shape (31px hidden bottom bar above a real payment-method row) - added a test asserting it now fails explicitly with no click, verifying the payment row is never touched. Added a second test where the payment row is itself the bottom-most clickable row (no submit region at all) and must be rejected by its own alias match. Fixed readySheet() (used by the address-save-wait test) to stop compounding liveShapedSheet()'s own hidden 31px placeholder with a separate real submit button - the two used to tie and could flip which one the rightmost tie-break picked; it now builds the same shape directly with submit="none" so "r/final" is the sole, unambiguous bottom row. Updated one diagnostic key assertion (tie= -> rowTie=) for the row-level tie-break. Every other #335 test (legacy 提交订单, 570 structure + ReFindingDriver re-find/climb, payment-word blocked, outside-panel-container, no-determinable-container) passed unmodified. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NTDbDcwbDw1TSAcE6wfh2F
2026-09-23 10:57:52 +08:00
parent edc956c430
commit e82762e6b7
2 changed files with 117 additions and 50 deletions
@@ -777,26 +777,37 @@ class PurchaseLiveAutomation(
    }

    /**
-     * #335 (2nd revision, task 594 real-device failure): the returned target must be a
-     * node with a NON-BLANK OWN LABEL, not the (often label-less) clickable container.
-     * [PurchaseUiDriver.clickFresh] re-finds its target live by
-     * `preferredOrDescendantLabel() == target.label && className && center±32` and only
-     * THEN climbs to the nearest clickable ancestor and clicks it (see
-     * `GoAutoAccessibilityService.clickFreshDetailed`). A label-less clickable
-     * FrameLayout's [SnapshotNode.label] is `""`, but its live `preferredOrDescendantLabel()`
-     * digs into a child and returns real text — so the re-find never matches and the click
-     * silently fails as `TARGET_NOT_FOUND` right after the irreversible boundary (task 594:
-     * "大促价，" bottom bar selected correctly, click never happened). Aiming at the
-     * labelled leaf instead — the same convention already used by
-     * `ImageSearchCandidatePolicy.clickTargetInside` — lets the existing climb-to-ancestor
-     * logic land the tap on the right container.
+     * #335 (3rd revision, reviewer cross-check on real dumps — Samsung sample): a
+     * two-step "row, then leaf" pick, not a single whole-panel leaf scan. A whole-panel
+     * scan can pick a labelled leaf that lives in a DIFFERENT clickable row than the real
+     * bottom bar — on the Samsung sample the bottom bar is a label-less 31px FrameLayout
+     * whose only text node is zero-size, so a naive scan falls through to the next
+     * lowest labelled leaf, which sits in the PAYMENT-METHOD row above it, and the climb
+     * from there lands the click on "change payment method" instead of the order button.
     *
-     * Rule: within the recognized spec panel's container (#331 judgment, [PURCHASE_CONFIRMATION_PANEL_TYPES]),
-     * among visible/enabled/non-zero-size nodes with a non-blank own label that have a
-     * clickable ancestor (or are clickable themselves) of non-zero size, pick the one with
-     * the lowest bottom edge; ties go to the rightmost. The nearest clickable ancestor's
-     * subtree must not contain [SUBMIT_TARGET_BLOCKED_MARKERS]. [FINAL_SUBMIT_MARKERS] text
-     * match is diagnostic only, never required.
+     * Step 1 — row: among visible/enabled/non-zero-size CLICKABLE nodes inside the
+     * recognized panel's container (#331 judgment) with height ≤ [SUBMIT_ROW_MAX_HEIGHT_PERCENT]
+     * of screen height (same guard as 2bc624f, keeps a full-sheet/full-body wrapper from
+     * ever being "the row"), pick the one with the lowest bottom edge; ties go to the
+     * rightmost. This row — and only this row — may hold the order button. If its own
+     * subtree matches a payment-method alias (`textAliases.specPanel.paymentAreaAliases`,
+     * e.g. 微信支付/先用后付/支付方式) or [SUBMIT_TARGET_BLOCKED_MARKERS], fail explicitly;
+     * never fall back to a higher row — a higher row is never the order button either.
+     *
+     * Step 2 — leaf: within that row's own subtree (or the row itself), among
+     * visible/non-zero-size nodes with a NON-BLANK OWN LABEL whose nearest clickable
+     * ancestor is EXACTLY that row (not some nested sub-button, and not a higher row),
+     * pick the bottom-right-most one. [PurchaseUiDriver.clickFresh] re-finds its target
+     * live by `preferredOrDescendantLabel() == target.label && className && center±32`
+     * and only then climbs to the nearest clickable ancestor and clicks it (see
+     * `GoAutoAccessibilityService.clickFreshDetailed`); a label-less container's
+     * [SnapshotNode.label] is `""` but its live `preferredOrDescendantLabel()` digs into a
+     * child, so the re-find would always miss (task 594: TARGET_NOT_FOUND right after the
+     * irreversible boundary). Aiming at the labelled leaf — the same convention already
+     * used by `ImageSearchCandidatePolicy.clickTargetInside` — lets clickFresh's own
+     * climb-to-ancestor logic land the tap back on the row. If the row has no such leaf
+     * (Samsung: only a zero-size text node), fail explicitly (`bottom_row_unlabelled`) —
+     * never fall back to a higher row.
     */
    private fun finalSubmitTargets(snapshot: UiSnapshot): List<SnapshotNode> {
        fun hasArea(node: SnapshotNode) = node.bounds.width > 0 && node.bounds.height > 0
@@ -816,45 +827,55 @@ class PurchaseLiveAutomation(
            panelDiagnostic("bottomSubmit;outcome=no_container;type=${screen.specPanelType}")
            return emptyList()
        }
-        // A full-sheet or full-body wrapper container is sometimes clickable too (it can
-        // intercept touches); this bounds a *candidate leaf's own* size only as extra
-        // defense — a label-less wrapper is already excluded by requiring a non-blank own
-        // label below, but a stray, unrealistically large labelled node should not win either.
        val screenHeight = snapshot.nodes.filter { it.visible }.maxOfOrNull { it.bounds.bottom }?.coerceAtLeast(1) ?: 1
-        val candidates = snapshot.nodes.filter { node ->
-            node.visible && node.enabled && hasArea(node) && node.label.isNotEmpty() &&
+
+        // Step 1: the bottom-most clickable row.
+        val rowCandidates = snapshot.nodes.filter { node ->
+            node.visible && node.enabled && node.clickable && hasArea(node) &&
                node.bounds.height.toLong() * 100 <= screenHeight.toLong() * SUBMIT_ROW_MAX_HEIGHT_PERCENT &&
-                inside(node.bounds, container) &&
-                clickableAncestor(node)?.let(::hasArea) == true
+                inside(node.bounds, container)
        }.distinctBy { it.path }
-        if (candidates.isEmpty()) {
-            panelDiagnostic("bottomSubmit;outcome=no_candidates;type=${screen.specPanelType}")
+        if (rowCandidates.isEmpty()) {
+            panelDiagnostic("bottomSubmit;outcome=no_rows;type=${screen.specPanelType}")
            return emptyList()
        }
-        val maxBottom = candidates.maxOf { it.bounds.bottom }
-        val bottomRow = candidates.filter { it.bounds.bottom == maxBottom }
-        val tie = bottomRow.size > 1
-        val chosen = bottomRow.maxBy { it.bounds.right }
-        val ancestor = clickableAncestor(chosen)
-        if (ancestor == null) {
-            // Unreachable given the candidate filter above, kept as an explicit safety net.
-            panelDiagnostic("bottomSubmit;outcome=no_clickable_ancestor;tie=${tie.diagFlag()}")
-            return emptyList()
-        }
-        val subtreeLabels = (listOf(ancestor.label) + subtreeDescendants(ancestor, snapshot).map(SnapshotNode::label))
-            .joinToString("")
-        if (SUBMIT_TARGET_BLOCKED_MARKERS.any(subtreeLabels::contains)) {
+        val rowMaxBottom = rowCandidates.maxOf { it.bounds.bottom }
+        val rowBottomTied = rowCandidates.filter { it.bounds.bottom == rowMaxBottom }
+        val rowTie = rowBottomTied.size > 1
+        val row = rowBottomTied.maxBy { it.bounds.right }
+
+        val rowSubtreeLabels = (listOf(row.label) + subtreeDescendants(row, snapshot).map(SnapshotNode::label)).joinToString("")
+        val paymentAreaAliases = PurchaseRehearsalExecutor.DEFAULT_COLLECTOR.textAliases.specPanel.paymentAreaAliases
+        if (paymentAreaAliases.any(rowSubtreeLabels::contains) || SUBMIT_TARGET_BLOCKED_MARKERS.any(rowSubtreeLabels::contains)) {
            panelDiagnostic(
-                "bottomSubmit;outcome=payment_blocked;tie=${tie.diagFlag()};class=${ancestor.className};" +
-                    "w=${ancestor.bounds.width};h=${ancestor.bounds.height}",
+                "bottomSubmit;outcome=row_blocked;tie=${rowTie.diagFlag()};class=${row.className};" +
+                    "w=${row.bounds.width};h=${row.bounds.height}",
            )
            return emptyList()
        }
-        val hasPrice = PRICE_PRESENCE.containsMatchIn(subtreeLabels)
-        val legacyMarker = FINAL_SUBMIT_MARKERS.any { subtreeLabels == it || subtreeLabels.startsWith(it) }
+
+        // Step 2: the bottom-right-most labelled leaf whose nearest clickable ancestor is exactly this row.
+        val leafCandidates = (listOf(row) + subtreeDescendants(row, snapshot)).filter { node ->
+            node.visible && node.enabled && hasArea(node) && node.label.isNotEmpty() &&
+                clickableAncestor(node)?.path == row.path
+        }.distinctBy { it.path }
+        if (leafCandidates.isEmpty()) {
+            panelDiagnostic(
+                "bottomSubmit;outcome=bottom_row_unlabelled;tie=${rowTie.diagFlag()};class=${row.className};" +
+                    "w=${row.bounds.width};h=${row.bounds.height}",
+            )
+            return emptyList()
+        }
+        val leafMaxBottom = leafCandidates.maxOf { it.bounds.bottom }
+        val leafBottomTied = leafCandidates.filter { it.bounds.bottom == leafMaxBottom }
+        val leafTie = leafBottomTied.size > 1
+        val chosen = leafBottomTied.maxBy { it.bounds.right }
+
+        val hasPrice = PRICE_PRESENCE.containsMatchIn(rowSubtreeLabels)
+        val legacyMarker = FINAL_SUBMIT_MARKERS.any { rowSubtreeLabels == it || rowSubtreeLabels.startsWith(it) }
        panelDiagnostic(
-            "bottomSubmit;outcome=ok;tie=${tie.diagFlag()};leafClass=${chosen.className};" +
-                "ancestorClass=${ancestor.className};w=${ancestor.bounds.width};h=${ancestor.bounds.height};" +
+            "bottomSubmit;outcome=ok;rowTie=${rowTie.diagFlag()};leafTie=${leafTie.diagFlag()};" +
+                "leafClass=${chosen.className};rowClass=${row.className};w=${row.bounds.width};h=${row.bounds.height};" +
                "price=${hasPrice.diagFlag()};legacyMarker=${legacyMarker.diagFlag()};type=${screen.specPanelType}",
        )
        return listOf(chosen)
@@ -533,7 +533,7 @@ class SpecPanelRecognitionTest {

        // r/b is the rightmost of the tied bottom row, so it is the one clicked.
        assertEquals(listOf("大促价，¥16.9"), driver.clicked)
-        assertTrue(diagnostics.any { it.contains("outcome=ok") && it.contains("tie=1") })
+        assertTrue(diagnostics.any { it.contains("outcome=ok") && it.contains("rowTie=1") })
    }

    @Test
@@ -603,6 +603,44 @@ class SpecPanelRecognitionTest {
        assertEquals("r/sheet/submit", driver.clickedPath)
    }

+    @Test
+    fun `Samsung 31px bottom bar with only a zero-size label fails explicitly instead of clicking the payment row above it`() {
+        // #335 (3rd revision): reviewer cross-check on real dumps found that on this
+        // Samsung sample the bottom-most row is a 31px FrameLayout whose only text node
+        // is [0,0][0,0] (invisible), while the payment-method row directly above it has a
+        // real, visible label ("使用#微信支付，更换先用后付可0元下单"). A naive whole-panel
+        // leaf scan falls through to that payment row's text and the climb lands the click
+        // on "change payment method" — which must never happen. SpecPanelFixtures'
+        // `submit = "hidden"` branch already models exactly this shape.
+        val snapshot = SpecPanelFixtures.sheet(Sheet(submit = "hidden"))
+        val diagnostics = mutableListOf<String>()
+        val driver = StaticDriver(snapshot)
+        val error = runCatching {
+            PurchaseLiveAutomation(driver, pause = {}, panelDiagnostic = diagnostics::add).submitOrderOnce()
+        }.exceptionOrNull() as? PurchaseLiveException
+
+        assertEquals("PURCHASE_SUBMIT_TARGET_AMBIGUOUS", error?.code)
+        assertTrue(driver.clicked.isEmpty())
+        assertTrue(diagnostics.any { it.contains("outcome=bottom_row_unlabelled") })
+    }
+
+    @Test
+    fun `a payment-method row as the bottom-most clickable row is never a submit target`() {
+        // Without a submit region at all, the payment-method row ("使用#微信支付，更换先用
+        // 后付可0元下单") is itself the bottom-most clickable row. It must be rejected by
+        // its own alias match, not merely skipped in favor of something else.
+        val snapshot = SpecPanelFixtures.sheet(Sheet(submit = "none"))
+        val diagnostics = mutableListOf<String>()
+        val driver = StaticDriver(snapshot)
+        val error = runCatching {
+            PurchaseLiveAutomation(driver, pause = {}, panelDiagnostic = diagnostics::add).submitOrderOnce()
+        }.exceptionOrNull() as? PurchaseLiveException
+
+        assertEquals("PURCHASE_SUBMIT_TARGET_AMBIGUOUS", error?.code)
+        assertTrue(driver.clicked.isEmpty())
+        assertTrue(diagnostics.any { it.contains("outcome=row_blocked") })
+    }
+
    @Test
    fun `a clickable node outside the recognized panel container is never chosen even if it is bottom most`() {
        // A hand-built #331 REQUIRED_EVIDENCE panel ("r/panel", bounded, not full-screen)
@@ -659,7 +697,15 @@ class SpecPanelRecognitionTest {
    // --- Helpers -------------------------------------------------------------

    private fun readySheet(expected: String): UiSnapshot {
-        val base = SpecPanelFixtures.liveShapedSheet()
+        // #335 (3rd revision): [SpecPanelFixtures.liveShapedSheet] hardcodes a "hidden"
+        // 31px submit placeholder (its own separate `submit`-shape fixture concern,
+        // unrelated to this address-save-wait test). With the row-then-leaf submit-target
+        // rule that placeholder would tie with "r/final" on the bottom edge and can win
+        // the rightmost tie-break, so this helper builds the same shape directly with
+        // `submit = "none"` instead, leaving "r/final" as the only, unambiguous bottom row.
+        val base = SpecPanelFixtures.sheet(
+            Sheet(summary = false, listScrollable = false, sizeDimension = false, adjustButtons = false, submit = "none"),
+        )
        val extra = listOf(
            node("r/sheet/body/addr/a/saved", expected, NodeBounds(132, 460, 937, 480), parent = "r/sheet/body/addr/a"),
            node("r/final", "", NodeBounds(600, 2150, 1060, 2216), clickable = true, parent = "r"),