From ff6513b89c5fb28beb7a8d82f3a43f26230e6f1f Mon Sep 17 00:00:00 2001
From: PJ <ipriyanshujain@gmail.com>
Date: Sat, 6 Jun 2026 00:32:41 +0530
Subject: [PATCH] fix(runner): derive trace tap point from resolveCoordinates

stampSelectorTarget preferred possibly-stale action X/Y while dispatch
preferred the fresh tree-resolved center, so the trace could record a
different point than the one tapped. Both now share resolveCoordinates.
---
 internal/runner/runner.go      | 36 ++++++++++++++--------------------
 internal/runner/runner_test.go | 30 ++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+), 21 deletions(-)

diff --git a/internal/runner/runner.go b/internal/runner/runner.go
index a2d8237..383cbfa 100644
--- a/internal/runner/runner.go
+++ b/internal/runner/runner.go
@@ -760,29 +760,23 @@ func traceActionFor(action verifier.Action, tree *hierarchy.Tree) *trace.Action
 	return traceAction
 }
 
-// stampSelectorTarget mirrors applyAction's coordinate-resolution rule so the
-// trace records the same point the runner taps.
+// stampSelectorTarget records the element bounds the selector resolved to and
+// derives the tap point through resolveCoordinates, the same rule applyAction
+// dispatches with, so the trace can never record a different point than the
+// one tapped.
 func stampSelectorTarget(traceAction *trace.Action, action verifier.Action, tree *hierarchy.Tree) {
-	if action.X > 0 && action.Y > 0 {
-		traceAction.TapPoint = &trace.PointRecord{X: action.X, Y: action.Y}
-		return
+	if action.On != "" && tree != nil {
+		if element := tree.Find(action.On); element != nil {
+			bounds := element.Bounds
+			traceAction.ResolvedBounds = &trace.BoundsRecord{
+				X:      bounds.Left,
+				Y:      bounds.Top,
+				Width:  bounds.Width(),
+				Height: bounds.Height(),
+			}
+		}
 	}
-	if tree == nil || action.On == "" {
-		return
-	}
-	element := tree.Find(action.On)
-	if element == nil {
-		return
-	}
-	bounds := element.Bounds
-	traceAction.ResolvedBounds = &trace.BoundsRecord{
-		X:      bounds.Left,
-		Y:      bounds.Top,
-		Width:  bounds.Width(),
-		Height: bounds.Height(),
-	}
-	x, y := bounds.Center()
-	if x > 0 && y > 0 {
+	if x, y, ok := resolveCoordinates(action, tree); ok {
 		traceAction.TapPoint = &trace.PointRecord{X: x, Y: y}
 	}
 }
diff --git a/internal/runner/runner_test.go b/internal/runner/runner_test.go
index 1ee7eb6..f7fbbcb 100644
--- a/internal/runner/runner_test.go
+++ b/internal/runner/runner_test.go
@@ -522,6 +522,36 @@ func TestRunner_StampsHierarchyResolvedBoundsAndResiduals(t *testing.T) {
 	}
 }
 
+// TestTraceActionFor_StaleCoordinatesDoNotOverrideTreeCenter pins the stamp
+// to applyAction's resolution rule: when On resolves in the tree, the trace
+// tap point must be the tree center even if the action carries stale X/Y
+// from an earlier tick.
+func TestTraceActionFor_StaleCoordinatesDoNotOverrideTreeCenter(t *testing.T) {
+	tree, err := hierarchy.Parse(`{"attributes":{"resource-id":"root","bounds":"[0,0,1080,2340]"},"children":[
+		{"attributes":{"resource-id":"next","bounds":"[100,200,300,400]"},"children":[]}
+	]}`)
+	if err != nil {
+		t.Fatalf("Parse: %v", err)
+	}
+	action := verifier.Action{Kind: verifier.ActionKindTap, On: "id:next", X: 50, Y: 60}
+
+	traceAction := traceActionFor(action, tree)
+	if traceAction.TapPoint == nil {
+		t.Fatal("expected a tap point")
+	}
+	if traceAction.TapPoint.X != 200 || traceAction.TapPoint.Y != 300 {
+		t.Errorf("tap point = (%d,%d), want tree center (200,300)",
+			traceAction.TapPoint.X, traceAction.TapPoint.Y)
+	}
+	if traceAction.ResolvedBounds == nil {
+		t.Fatal("expected resolved bounds")
+	}
+	if traceAction.ResolvedBounds.X != 100 || traceAction.ResolvedBounds.Y != 200 {
+		t.Errorf("resolved bounds origin = (%d,%d), want (100,200)",
+			traceAction.ResolvedBounds.X, traceAction.ResolvedBounds.Y)
+	}
+}
+
 func TestRunner_LogsWaitForIdleDriverErrors(t *testing.T) {
 	state := newHarness(t)
 	state.mock.Failures[mockdriver.ActionWaitForIdle] = errors.New("sidecar lost gRPC stream")