fix(verifier): dedup candidates by what they execute, not how they read

The dedup key was the rendered description, which embeds the label, so two
distinct controls sharing a visible label collapsed to one entry and the
survivor carried the first one's action. The second control was not mislabelled,
it was absent from the candidate list, so no policy could reach it. Two
scrollable containers collapsed the same way, leaving the second unscrollable.

The key is now the executable Action struct itself plus whether the model
supplies the typed text, so a new Action field cannot silently fall out of it.
Descriptions may now repeat; the numbering disambiguates and the echo guard is
index-anchored, not description-anchored.

This also makes the label source a pure observation-channel change. It was not
one before: the label fed the dedup key, so the two arms of the labelling
factor enumerated different-sized candidate lists, in both directions depending
on the screen.

Claude-Session: https://claude.ai/code/session_01A5KmftdEJ49A9z5mF5ESrX

internal/verifier/llm.go

@@ -113,7 +113,9 @@ type ActionCandidate struct {
 	// Kind is the resulting action kind.
 	Kind ActionKind
 	// Description is the rendered action shown to the model and echoed back as
-	// chosen_action, e.g. `Tap "Add credit"`. Dedup keys on it, so it is unique.
+	// chosen_action, e.g. `Tap "Add credit"`. Two entries may render the same
+	// when the screen holds two controls a user reads alike; Index is what tells
+	// them apart, and it is what the model picks by.
 	Description string
 	// Label is the target label the selected LabelSource named (empty for
 	// gestures).
@@ -162,7 +164,8 @@ const (
 // SAME tree the seeded picker draws: weighted branches recurse (accumulating the
 // selection probability), authored actions()/whenRoute leaves are called once
 // for their concrete actions, and builtin verbs come straight from the picker's
-// own enumeration. Identical descriptions dedup, summing weight.
+// own enumeration. Candidates that would execute the same action dedup, summing
+// weight; two controls that merely read alike stay two entries.
 //
 // labelSource selects the channel each target is named by. An unrecognized
 // value (including the zero value) names targets by visible text; the CLI
@@ -491,20 +494,33 @@ func (v *Verifier) enumerateBuiltin(verb string) ([]builtinCandidate, error) {
 	return entries, nil
 }
 
-// finalizeCandidates renders each candidate's description, dedups identical
-// descriptions (summing weight), numbers the survivors 1..N, and rounds the
-// accumulated probability into a percentage Weight.
+// candidateIdentity is what a candidate would DO. Two candidates sharing it are
+// the same action reached through two paths of the action tree, so folding them
+// into one numbered entry loses nothing; two that differ are different actions
+// however alike they read, so folding them would put one of them out of reach.
+// llmText is part of it because it decides where the typed text comes from: the
+// model writes it for a builtin typing candidate, while an authored one replays
+// the value already sitting in Action.Text.
+type candidateIdentity struct {
+	action  Action
+	llmText bool
+}
+
+// finalizeCandidates renders each candidate's description, dedups by what the
+// candidate executes (summing weight), numbers the survivors 1..N, and rounds
+// the accumulated probability into a percentage Weight.
 func finalizeCandidates(raw []ActionCandidate) []ActionCandidate {
-	seen := make(map[string]int, len(raw))
+	seen := make(map[candidateIdentity]int, len(raw))
 	result := make([]ActionCandidate, 0, len(raw))
 	for _, candidate := range raw {
 		candidate.Description = describeCandidate(candidate)
-		if index, ok := seen[candidate.Description]; ok {
+		identity := candidateIdentity{action: candidate.Action, llmText: candidate.LLMText}
+		if index, ok := seen[identity]; ok {
 			result[index].prob += candidate.prob
 			result[index].Weighted = result[index].Weighted || candidate.Weighted
 			continue
 		}
-		seen[candidate.Description] = len(result)
+		seen[identity] = len(result)
 		result = append(result, candidate)
 	}
 	for i := range result {
@@ -517,8 +533,11 @@ func finalizeCandidates(raw []ActionCandidate) []ActionCandidate {
 }
 
 // describeCandidate renders the plain, echo-friendly description shown in the
-// numbered list. It is the dedup key, so it must be stable and unique per
-// distinct action.
+// numbered list. It is display only: dedup keys on the action, so two entries
+// may read alike without merging and Index is what separates them. Do not add an
+// ordinal or a coordinate to pull those apart: this string IS the observation
+// channel a labelling experiment varies, so a disambiguator here would name a
+// target through a channel the label source deliberately withholds.
 func describeCandidate(candidate ActionCandidate) string {
 	switch candidate.Kind {
 	case ActionKindTap:
@@ -538,10 +557,11 @@ func describeCandidate(candidate ActionCandidate) string {
 	case ActionKindScroll:
 		return "Scroll " + candidate.Direction
 	case ActionKindSwipe:
-		// A swipe carries endpoints and no selector, so the coordinates are what
-		// keep two swipes distinct. The label is prepended when the origin
-		// element has one, because "swipe that row" is the interaction a model
-		// reaches for and a bare pair of points does not say which row.
+		// A swipe is a drag across the screen, so where it runs is the whole of
+		// what it does and a reader needs the endpoints to picture it. The label
+		// is prepended when the origin element has one, because "swipe that row"
+		// is the interaction a model reaches for and a bare pair of points does
+		// not say which row.
 		where := fmt.Sprintf("from (%d,%d) to (%d,%d)",
 			candidate.Action.FromX, candidate.Action.FromY,
 			candidate.Action.ToX, candidate.Action.ToY)

internal/verifier/llm_test.go

@@ -41,6 +41,18 @@ const labelChannelTreeJSON = `{
   ]
 }`
 
+// sharedLabelTreeJSON is a list where two rows read exactly the same to a user
+// ("Delete") while the app tells them apart by identifier. It is the shape a
+// list of removable items has in any real app.
+const sharedLabelTreeJSON = `{
+  "attributes": {"bounds": "[0,0,1080,2400]"},
+  "children": [
+    {"attributes": {"resource-id": "delete_alpha", "text": "Delete", "bounds": "[0,100,1080,200]"}, "clickable": true, "enabled": true, "children": []},
+    {"attributes": {"resource-id": "delete_beta", "text": "Delete", "bounds": "[0,200,1080,300]"}, "clickable": true, "enabled": true, "children": []},
+    {"attributes": {"resource-id": "checkout", "text": "Checkout", "bounds": "[0,400,1080,500]"}, "clickable": true, "enabled": true, "children": []}
+  ]
+}`
+
 // enumVerifier loads a spec whose actions root is the given plain-object graph
 // and stages the given tree, so Candidates walks a controlled action tree. The
 // spec is bundled with the goja runtime entry because the model arm reads the
@@ -125,12 +137,12 @@ func TestCandidatesIdentifierChannelFallsBackToClassThenBareControl(t *testing.T
 	}
 }
 
-// TestCandidatesIdentifierChannelMergesControlsItCannotTellApart pins the cost
-// of the channel rather than a defect in it: dedup keys on the rendered line, so
-// two identifier-less controls of one class arrive as ONE numbered entry and the
-// model can only reach the first. The list the identifier arm picks from is
-// therefore shorter than the text arm's on the same screen.
-func TestCandidatesIdentifierChannelMergesControlsItCannotTellApart(t *testing.T) {
+// TestCandidatesIdentifierChannelKeepsControlsItCannotNameApartReachable is the
+// cost of the channel, bounded: two identifier-less controls of one class read
+// the same in the numbered list, but they stay TWO entries, each carrying its
+// own action, so the model can act on either by number. A channel that renames
+// controls must never shrink the action space.
+func TestCandidatesIdentifierChannelKeepsControlsItCannotNameApartReachable(t *testing.T) {
 	text := enumVerifier(t, "{kind:'builtin', verb:'taps'}", labelChannelTreeJSON).
 		Candidates(LabelSourceVisibleText)
 	identifier := enumVerifier(t, "{kind:'builtin', verb:'taps'}", labelChannelTreeJSON).
@@ -139,15 +151,44 @@ func TestCandidatesIdentifierChannelMergesControlsItCannotTellApart(t *testing.T
 	if count(text, `Tap "Remember me"`) != 1 || count(text, `Tap "Stay signed in"`) != 1 {
 		t.Fatalf("the text channel should address both checkboxes, got %v", descriptions(text))
 	}
-	if got := count(identifier, `Tap "android.widget.CheckBox"`); got != 1 {
-		t.Errorf("the two checkboxes should merge into one line, got %d: %v", got, descriptions(identifier))
+	checkboxes := candidatesMatching(identifier, `Tap "android.widget.CheckBox"`)
+	if len(checkboxes) != 2 {
+		t.Fatalf("the two checkboxes should be two entries, got %d: %v",
+			len(checkboxes), descriptions(identifier))
 	}
-	if len(identifier) >= len(text) {
-		t.Errorf("identifier list (%d) should be shorter than the text list (%d): %v vs %v",
+	if checkboxes[0].Action == checkboxes[1].Action {
+		t.Errorf("both entries execute the same action: %+v", checkboxes[0].Action)
+	}
+	if len(identifier) != len(text) {
+		t.Errorf("identifier list (%d) and text list (%d) must offer the same actions: %v vs %v",
 			len(identifier), len(text), descriptions(identifier), descriptions(text))
 	}
 }
 
+// TestCandidatesReachBothControlsSharingOneVisibleLabel is the reachability
+// floor: two rows a user reads as the same word are two different controls, so
+// both get a number and the second number taps the second row. Dedup that keyed
+// on the rendered line dropped the second one, putting it out of reach of any
+// prompt or policy.
+func TestCandidatesReachBothControlsSharingOneVisibleLabel(t *testing.T) {
+	candidates := enumVerifier(t, "{kind:'builtin', verb:'taps'}", sharedLabelTreeJSON).
+		Candidates(LabelSourceVisibleText)
+
+	deletes := candidatesMatching(candidates, `Tap "Delete"`)
+	if len(deletes) != 2 {
+		t.Fatalf("want both Delete rows reachable, got %d: %v", len(deletes), descriptions(candidates))
+	}
+	if got := deletes[0].Action.On; got != "id:delete_alpha" {
+		t.Errorf("first entry targets %q, want id:delete_alpha", got)
+	}
+	if got := deletes[1].Action.On; got != "id:delete_beta" {
+		t.Errorf("second entry targets %q, want id:delete_beta", got)
+	}
+	if deletes[0].Index == deletes[1].Index {
+		t.Errorf("both entries share number %d, so the model cannot address them apart", deletes[0].Index)
+	}
+}
+
 // TestCandidatesVisibleTextFallsBackToTheIdentifier is where the two channels
 // agree: a control carrying nothing readable is named by its identifier in both,
 // so a screen built entirely from such controls is one cell, not two.
@@ -175,25 +216,56 @@ func TestCandidatesTypingLabelFollowsTheLabelSource(t *testing.T) {
 	}
 }
 
-// TestLabelSourceChangesOnlyTheDescription is the claim the factorial rests on:
-// the channel renames the target and does nothing else, so a difference in
-// defect yield cannot come from the two arms executing different actions.
+// TestLabelSourceChangesOnlyTheDescription is the claim the labelling factorial
+// rests on: the channel renames every target and does nothing else. Both arms
+// enumerate the same candidates, in the same order, at the same weights,
+// carrying the same executable actions; the description and the label are the
+// only things that move. Anything else and the two cells would be picking from
+// different action spaces, so a difference in defect yield could not be
+// attributed to how the controls were named.
 func TestLabelSourceChangesOnlyTheDescription(t *testing.T) {
-	text := enumVerifier(t, "{kind:'builtin', verb:'taps'}", labelChannelTreeJSON).
-		Candidates(LabelSourceVisibleText)
-	identifier := enumVerifier(t, "{kind:'builtin', verb:'taps'}", labelChannelTreeJSON).
-		Candidates(LabelSourceResourceID)
-
-	readable, ok := findCandidate(text, `Tap "Add credit"`)
-	if !ok {
-		t.Fatalf("missing the text-labelled tap: %v", descriptions(text))
+	const everyLabelledVerb = `{kind:'weighted', branches:[
+      [1,{kind:'builtin',verb:'taps'}],
+      [1,{kind:'builtin',verb:'typing'}],
+      [1,{kind:'builtin',verb:'swipes'}]
+    ]}`
+	fixtures := []struct {
+		name string
+		tree string
+	}{
+		{"identifiers collide", labelChannelTreeJSON},
+		{"visible text collides", sharedLabelTreeJSON},
 	}
-	named, ok := findCandidate(identifier, `Tap "add_credit_button"`)
-	if !ok {
-		t.Fatalf("missing the identifier-labelled tap: %v", descriptions(identifier))
+	withoutNames := func(candidate ActionCandidate) ActionCandidate {
+		candidate.Description = ""
+		candidate.Label = ""
+		return candidate
 	}
-	if readable.Action != named.Action {
-		t.Errorf("same control, different action:\n text=%+v\n   id=%+v", readable.Action, named.Action)
+	for _, fixture := range fixtures {
+		t.Run(fixture.name, func(t *testing.T) {
+			text := enumVerifier(t, everyLabelledVerb, fixture.tree).Candidates(LabelSourceVisibleText)
+			identifier := enumVerifier(t, everyLabelledVerb, fixture.tree).Candidates(LabelSourceResourceID)
+			if len(text) == 0 {
+				t.Fatal("fixture yielded no candidates")
+			}
+			if len(text) != len(identifier) {
+				t.Fatalf("different action spaces: %d text candidates vs %d identifier ones:\n%v\n%v",
+					len(text), len(identifier), descriptions(text), descriptions(identifier))
+			}
+			renamed := false
+			for i := range text {
+				if withoutNames(text[i]) != withoutNames(identifier[i]) {
+					t.Errorf("candidate %d differs beyond its name:\n text=%+v\n   id=%+v",
+						i+1, text[i], identifier[i])
+				}
+				if text[i].Description != identifier[i].Description {
+					renamed = true
+				}
+			}
+			if !renamed {
+				t.Error("no candidate was renamed, so this fixture does not exercise the channel")
+			}
+		})
 	}
 }
 
@@ -429,6 +501,16 @@ func descriptions(candidates []ActionCandidate) []string {
 	return out
 }
 
+func candidatesMatching(candidates []ActionCandidate, description string) []ActionCandidate {
+	var matched []ActionCandidate
+	for _, candidate := range candidates {
+		if candidate.Description == description {
+			matched = append(matched, candidate)
+		}
+	}
+	return matched
+}
+
 func count(candidates []ActionCandidate, description string) int {
 	n := 0
 	for _, candidate := range candidates {