diff --git a/internal/hierarchy/hierarchy.go b/internal/hierarchy/hierarchy.go index 9972cb4..c54a052 100644 --- a/internal/hierarchy/hierarchy.go +++ b/internal/hierarchy/hierarchy.go @@ -272,6 +272,25 @@ func elementFromNode(node *treeNodeJSON) *Element { return element } +// Transitional reports more than one resource id ending in "Screen": the marker +// of a Compose NavHost mid cross-fade, where the source and destination route +// composables are both alive in a collapsed, mid-animation layout. +func (t *Tree) Transitional() bool { + if t == nil { + return false + } + screens := 0 + for _, element := range t.Elements { + if strings.HasSuffix(element.ResourceID, "Screen") { + screens++ + if screens > 1 { + return true + } + } + } + return false +} + // Find returns the first element matching the selector, or nil. func (t *Tree) Find(selector string) *Element { node := t.FindNode(selector) diff --git a/internal/hierarchy/hierarchy_test.go b/internal/hierarchy/hierarchy_test.go index 97b3286..91abd4c 100644 --- a/internal/hierarchy/hierarchy_test.go +++ b/internal/hierarchy/hierarchy_test.go @@ -1006,3 +1006,32 @@ func TestParseBoundsRejectsBadInput(t *testing.T) { } } } + +// Bug class: a NavHost cross-fade carries two route-level *Screen ids at once; +// both the runner's re-fetch guard and the LLM's candidate enumeration depend on +// spotting it, and neither must flag a settled single-screen tree. +func TestTreeTransitional(t *testing.T) { + multi, err := Parse(`{"attributes":{"resource-id":"root"},"children":[ + {"attributes":{"resource-id":"AddAccountScreen"},"children":[]}, + {"attributes":{"resource-id":"HomeScreen"},"children":[]} + ]}`) + if err != nil { + t.Fatal(err) + } + if !multi.Transitional() { + t.Error("expected multi-screen tree to be flagged as transitional") + } + + single, err := Parse(`{"attributes":{"resource-id":"HomeScreen"},"children":[]}`) + if err != nil { + t.Fatal(err) + } + if single.Transitional() { + t.Error("single-screen tree must not be flagged as transitional") + } + + var nilTree *Tree + if nilTree.Transitional() { + t.Error("nil tree must not be flagged as transitional") + } +} diff --git a/internal/runner/runner.go b/internal/runner/runner.go index 86a3d47..66462aa 100644 --- a/internal/runner/runner.go +++ b/internal/runner/runner.go @@ -878,7 +878,7 @@ retryLoop: tree, err = hierarchy.Parse(hierarchyJSON) pngBytes = image.PNG } - if err != nil || !isTransitionalHierarchy(tree) { + if err != nil || !tree.Transitional() { break } // A tree unchanged since the previous attempt is a settled state @@ -909,27 +909,6 @@ retryLoop: return tree, pngBytes, transitional, err } -// isTransitionalHierarchy returns true when the tree carries more than one -// resource-id ending in "Screen" - the marker of a Compose NavHost mid -// cross-fade where both source and destination route composables are alive. -// Mirrors the sidecar's stabilitySnapshot heuristic so runner-side rejection -// stays consistent with the settle poll. -func isTransitionalHierarchy(tree *hierarchy.Tree) bool { - if tree == nil { - return false - } - screens := 0 - for _, element := range tree.Elements { - if strings.HasSuffix(element.ResourceID, "Screen") { - screens++ - if screens > 1 { - return true - } - } - } - return false -} - func traceActionFor(action verifier.Action, tree *hierarchy.Tree) *trace.Action { traceAction := &trace.Action{Kind: string(action.Kind), X: action.X, Y: action.Y} switch action.Kind { diff --git a/internal/runner/runner_test.go b/internal/runner/runner_test.go index db7c7fa..31fecaa 100644 --- a/internal/runner/runner_test.go +++ b/internal/runner/runner_test.go @@ -1118,34 +1118,6 @@ func TestRunner_OneScreenshotPerStep(t *testing.T) { } } -// TestIsTransitionalHierarchy_DetectsMultipleScreens covers the runner-side -// guard that re-fetches when the hierarchy still carries two route-level -// *Screen ids - the NavHost cross-fade signature. -func TestIsTransitionalHierarchy_DetectsMultipleScreens(t *testing.T) { - multi, err := hierarchy.Parse(`{"attributes":{"resource-id":"root"},"children":[ - {"attributes":{"resource-id":"AddAccountScreen"},"children":[]}, - {"attributes":{"resource-id":"HomeScreen"},"children":[]} - ]}`) - if err != nil { - t.Fatal(err) - } - if !isTransitionalHierarchy(multi) { - t.Error("expected multi-screen tree to be flagged as transitional") - } - - single, err := hierarchy.Parse(`{"attributes":{"resource-id":"HomeScreen"},"children":[]}`) - if err != nil { - t.Fatal(err) - } - if isTransitionalHierarchy(single) { - t.Error("single-screen tree must not be flagged as transitional") - } - - if isTransitionalHierarchy(nil) { - t.Error("nil tree must not be flagged as transitional") - } -} - // TestRunner_StableTransitionalTreeIsVerified feeds a driver whose hierarchy // constantly carries two route-level *Screen ids but never changes between // retry attempts. Such a tree is a settled state that merely matches the diff --git a/internal/verifier/llm.go b/internal/verifier/llm.go index a090184..607dff7 100644 --- a/internal/verifier/llm.go +++ b/internal/verifier/llm.go @@ -130,22 +130,6 @@ type ActionCandidate struct { prob float64 } -// transitionalTree reports a NavHost cross-fade: more than one route *Screen -// alive at once. Mirrors runner.isTransitionalHierarchy so the LLM rejects the -// same frames the verifier's transitional handling does. -func transitionalTree(tree *hierarchy.Tree) bool { - screens := 0 - for _, element := range tree.Elements { - if strings.HasSuffix(element.ResourceID, "Screen") { - screens++ - if screens > 1 { - return true - } - } - } - return false -} - // verbActionKind maps a picker verb to the action kind it dispatches. func verbActionKind(verb string) ActionKind { switch verb { @@ -188,12 +172,10 @@ func (v *Verifier) Candidates() []ActionCandidate { if v.lastTree == nil { return nil } - // A cross-fade frame carries more than one route *Screen at once (a NavHost - // mid-transition); its layout is mid-animation, often in a collapsed + // A cross-fade frame's layout is mid-animation, often in a collapsed // coordinate space, so acting on it taps garbage (e.g. the soft keyboard). - // Skip it so the LLM re-observes a settled frame next step. Mirrors the - // runner's isTransitionalHierarchy. - if transitionalTree(v.lastTree) { + // Skip it so the LLM re-observes a settled frame next step. + if v.lastTree.Transitional() { return nil } root := v.runtime.GlobalObject().Get("actions")