Skip to content

Commit b715348

Browse files
committed
fix(exploration): normalize displayLabel keys, score-first ordering, persistence compat
1 parent 0eb8ce7 commit b715348

8 files changed

Lines changed: 176 additions & 63 deletions

CONTRIBUTING.md

Lines changed: 118 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -164,9 +164,10 @@ mirroir-mcp/
164164
│ │ └── ProcessExtensions.swift # Timeout-aware Process.wait
165165
│ │
166166
│ └── FakeMirroring/ # Test double app for CI (not a mock — a real macOS app)
167-
│ ├── main.swift # Entry point
168-
│ ├── FakeScreenDrawing.swift # Renders OCR-detectable text labels
169-
│ └── Scenarios.swift # Screen scenarios for integration tests
167+
│ ├── main.swift # Entry point, window setup, input handling
168+
│ ├── FakeScreenDrawing.swift # Renders OCR-detectable text labels, cards, tab bars
169+
│ ├── Scenarios.swift # Screen scenarios + NavigationMap for tap routing
170+
│ └── HealthScenarios.swift # Health-related scenarios (extracted for file size)
170171
171172
├── Tests/
172173
│ ├── MCPServerTests/ # XCTest — server routing, tool handlers, exploration (71 files)
@@ -390,6 +391,120 @@ All timing and numeric constants can be overridden via environment variables. Th
390391
|----------|---------|-------------|
391392
| `IPHONE_KEYBOARD_LAYOUT` | *(not set)* | Opt-in non-US keyboard layout for character translation (e.g., `Canadian-CSA` or `com.apple.keylayout.Canadian-CSA`). When unset, US QWERTY keycodes are sent. |
392393

394+
## FakeMirroring
395+
396+
FakeMirroring is a real macOS app that stands in for iPhone Mirroring during testing. It renders OCR-detectable text at known positions, responds to CGEvent taps, and supports scrolling — everything the real mirroring window does, without needing a physical iPhone.
397+
398+
### Build & Run
399+
400+
```bash
401+
swift build -c release --product FakeMirroring
402+
./scripts/package-fake-app.sh
403+
open .build/release/FakeMirroring.app
404+
```
405+
406+
The app window is 410x898pt (matching iPhone screen dimensions) and floats above other windows so CGEvent taps always land on it.
407+
408+
### Scenarios
409+
410+
FakeMirroring renders different screen layouts via **scenarios**. Switch scenarios from the Scenario menu or programmatically via `bridge.triggerMenuAction(menu: "Scenario", item: "Settings")`.
411+
412+
Key scenarios:
413+
414+
| Scenario | Content | Navigation |
415+
|----------|---------|------------|
416+
| `settings` | 6 rows with chevrons (General, Privacy, etc.) | General → detail, Notifications → notifications |
417+
| `detailWithBack` | Detail screen with `<` back button | `<` → back to source |
418+
| `healthSummary` | 3 viewports: cards + setup rows + articles | Activity/Workouts/Steps → detailWithBack |
419+
| `scrollableList` | 20 rows at 60pt spacing (scroll testing) | General → detail |
420+
| `feed` | Instagram-style posts with images | Tab bar navigation |
421+
422+
### Adding a Scenario
423+
424+
1. Add a case to `FakeScenario` enum in `Scenarios.swift`
425+
2. Add a `static func myScenario() -> ScenarioData` in the appropriate file (extract to a new file if `Scenarios.swift` is near 500 lines)
426+
3. Wire it in `ScenarioContent.data(for:)` switch
427+
4. Add tap routing in `NavigationMap.destination(from:tapping:)` — return the target scenario for each tappable label, or `nil` for dead taps
428+
429+
`ScenarioData` supports: `rows` (label + chevron), `cards` (Health-style summary cards), `plainTexts`, `buttons`, `placeholders`, and `hasTabBar` / `hasBackChevron`.
430+
431+
### NavigationMap
432+
433+
`NavigationMap.destination(from: scenario, tapping: label)` defines what happens when a label is tapped. Returns the target `FakeScenario` for navigation, or `nil` if the tap is a dead tap (no screen change). The BFS explorer uses this to discover new screens during integration tests.
434+
435+
### Input Handling
436+
437+
FakeMirroring handles: mouse clicks (tap), `scrollWheel` (swipe/scroll), `mouseDragged` (drag), long press (0.4s threshold), double tap (0.3s gap), and `keyDown` (text field typing). Hit regions are computed from rendered element positions. The `AlwaysAcceptingWindow` subclass accepts mouse events even when not the key window, so CGEvent-posted taps work during integration tests.
438+
439+
## Integration Tests
440+
441+
Integration tests in `Tests/IntegrationTests/` run real OCR against FakeMirroring's rendered text. They exercise the full pipeline: OCR → coordinate mapping → CGEvent tap → verify screen change.
442+
443+
### Running
444+
445+
```bash
446+
# Build and launch FakeMirroring first
447+
swift build -c release --product FakeMirroring
448+
./scripts/package-fake-app.sh
449+
open .build/release/FakeMirroring.app
450+
451+
# Run integration tests (FakeMirroring must be visible)
452+
swift test --filter IntegrationTests
453+
454+
# Run a single test
455+
swift test --filter BFSExplorationIntegrationTests/testMultiViewportExploration
456+
```
457+
458+
Integration tests are **skipped in CI** (`swift test --skip IntegrationTests`) because they require a visible macOS window and CGEvent access. They run locally before merging.
459+
460+
### Test Pattern
461+
462+
Every integration test follows this pattern:
463+
464+
```swift
465+
override func setUpWithError() throws {
466+
try IntegrationTestHelper.ensureFakeMirroringRunning()
467+
bridge = MirroringBridge(bundleID: IntegrationTestHelper.fakeBundleID)
468+
guard IntegrationTestHelper.ensureWindowReady(bridge: bridge) else {
469+
throw IntegrationTestError.windowNotCapturable
470+
}
471+
describer = ScreenDescriber(bridge: bridge, capture: ScreenCapture(bridge: bridge))
472+
input = InputSimulation(bridge: bridge)
473+
474+
_ = bridge.triggerMenuAction(menu: "Scenario", item: "Settings")
475+
usleep(500_000)
476+
}
477+
478+
override func tearDown() {
479+
// Restore default scenario for other tests
480+
_ = bridge?.triggerMenuAction(menu: "Scenario", item: "Settings")
481+
usleep(500_000)
482+
}
483+
```
484+
485+
Tests create `ScreenDescriber`, `InputSimulation`, and `ExplorationSession` directly — no MCP transport needed.
486+
487+
### Testing BFS Exploration
488+
489+
Two approaches, use the right one:
490+
491+
| Approach | When to use | Speed |
492+
|----------|-------------|-------|
493+
| **Unit tests** (`MockExplorerDescriber`) | Testing scroll logic, plan building, action counters, specific code paths | Fast (~2s) |
494+
| **Integration tests** (FakeMirroring) | Testing full exploration loop with real OCR, tap routing, backtracking | Slow (~2min) |
495+
496+
Unit test mocks return a pre-defined sequence of screens. Integration tests use real OCR output that varies slightly between runs. Use `seed: 42` for deterministic tap ordering in integration tests.
497+
498+
## Component Skills
499+
500+
Component definitions are `.md` files that describe iOS UI patterns (table rows, summary cards, modal sheets). The BFS explorer matches OCR elements against these definitions to decide what to tap.
501+
502+
Definitions live in the sibling [mirroir-skills](https://github.com/jfarcand/mirroir-skills) repo at `components/ios/`. They're loaded at runtime from `~/.mirroir-mcp/skills/components/ios/` or `<cwd>/.mirroir-mcp/skills/components/ios/` or `../mirroir-skills/components/ios/`.
503+
504+
Each definition has: Match Rules (zone, element count, chevron/numeric patterns), Interaction (click target, expected result), Exploration (explorable flag, role, priority), and Grouping (row absorption).
505+
506+
Test a definition against the current live screen with `calibrate_component`. See [Component Detection](docs/components.md) for the full format.
507+
393508
## Code Conventions
394509

395510
### File Headers

Sources/mirroir-mcp/ComponentSkillParser.swift

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -414,10 +414,20 @@ enum ComponentSkillParser {
414414
_ text: String, interaction: ComponentInteraction
415415
) -> ComponentExploration {
416416
let kv = extractKeyValues(from: text)
417+
let roleStr = kv["role"] ?? "depth_navigation"
418+
let priorityStr = kv["priority"] ?? "normal"
419+
if kv["role"] != nil && ExplorationRole(rawValue: roleStr) == nil {
420+
DebugLog.log("components",
421+
"WARNING: invalid exploration role '\(roleStr)' — defaulting to depth_navigation")
422+
}
423+
if kv["priority"] != nil && ExplorationPriority(rawValue: priorityStr) == nil {
424+
DebugLog.log("components",
425+
"WARNING: invalid exploration priority '\(priorityStr)' — defaulting to normal")
426+
}
417427
return ComponentExploration(
418428
explorable: parseBool(kv["explorable"]) ?? interaction.clickable,
419-
role: ExplorationRole(rawValue: kv["role"] ?? "depth_navigation") ?? .depthNavigation,
420-
priority: ExplorationPriority(rawValue: kv["priority"] ?? "normal") ?? .normal
429+
role: ExplorationRole(rawValue: roleStr) ?? .depthNavigation,
430+
priority: ExplorationPriority(rawValue: priorityStr) ?? .normal
421431
)
422432
}
423433

Sources/mirroir-mcp/GraphPathFinder.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,7 @@ enum GraphPathFinder {
212212
from path: [NavigationEdge],
213213
snapshot: GraphSnapshot
214214
) -> String {
215-
let labels = path.map(\.elementText).filter { !$0.isEmpty }
215+
let labels = path.map(\.displayLabel).filter { !$0.isEmpty }
216216
if labels.isEmpty { return "exploration" }
217217

218218
// Try landmark-based naming for longer paths

Sources/mirroir-mcp/GraphPersistence.swift

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,8 @@ struct SerializableEdge: Codable, Sendable {
9494
let toFingerprint: String
9595
let actionType: String
9696
let elementText: String
97-
let displayLabel: String
97+
/// Optional for backward compatibility: v1 graphs lack displayLabel.
98+
let displayLabel: String?
9899
let edgeType: String
99100
/// Learned action-value estimate. Optional for backward compatibility with v1 graphs.
100101
let qValue: Double?
@@ -115,7 +116,7 @@ struct SerializableEdge: Codable, Sendable {
115116
toFingerprint: toFingerprint,
116117
actionType: actionType,
117118
elementText: elementText,
118-
displayLabel: displayLabel,
119+
displayLabel: displayLabel ?? elementText,
119120
edgeType: EdgeType(rawValue: edgeType) ?? .push,
120121
qValue: qValue ?? 1.0
121122
)
@@ -133,8 +134,8 @@ struct SerializableGraph: Codable, Sendable {
133134
/// CEGAR refinement levels per fingerprint (optional for backward compatibility).
134135
let refinementLevels: [String: StateAbstraction.RefinementLevel]?
135136

136-
/// Current format version. Bump when the schema changes.
137-
static let currentVersion = 1
137+
/// Current format version. v2 added optional displayLabel on edges.
138+
static let currentVersion = 2
138139
}
139140

140141
/// Saves and loads NavigationGraph state to disk for incremental exploration.
@@ -203,9 +204,10 @@ enum GraphPersistence {
203204
decoder.dateDecodingStrategy = .iso8601
204205
let serializable = try decoder.decode(SerializableGraph.self, from: data)
205206

206-
guard serializable.version == SerializableGraph.currentVersion else {
207-
DebugLog.log("persistence", "Stale graph version \(serializable.version) " +
208-
"for \(bundleID), expected \(SerializableGraph.currentVersion)")
207+
// Accept v1 (legacy, no displayLabel on edges) and v2 (current).
208+
guard serializable.version >= 1 && serializable.version <= SerializableGraph.currentVersion else {
209+
DebugLog.log("persistence", "Unsupported graph version \(serializable.version) " +
210+
"for \(bundleID), expected 1–\(SerializableGraph.currentVersion)")
209211
return nil
210212
}
211213

Sources/mirroir-mcp/NavigationGraph.swift

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -336,6 +336,8 @@ final class NavigationGraph: @unchecked Sendable {
336336
}
337337

338338
/// Get unvisited elements for a screen, filtered by the visited set.
339+
/// Visited elements are stored by displayLabel (component-cleaned name),
340+
/// so this comparison uses displayLabel-compatible matching.
339341
func unvisitedElements(for fingerprint: String) -> [TapPoint] {
340342
lock.lock()
341343
defer { lock.unlock() }

Sources/mirroir-mcp/NavigationGraphVerification.swift

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -123,13 +123,15 @@ extension NavigationGraph {
123123

124124
/// Update Q-value for the most recent edge from a screen after observing the outcome.
125125
/// Called after `recordTransition` to learn from each action's result.
126+
/// Matches on displayLabel (the clean component label) since the BFS explorer
127+
/// passes displayLabel as the key for Q-updates and dead-edge marks.
126128
func updateQValue(fromFingerprint: String, elementText: String, result: TransitionResult) {
127129
lock.lock()
128130
defer { lock.unlock() }
129131

130-
// Find the edge to update (last matching edge in the adjacency list)
132+
// Find the edge to update by displayLabel (BFS passes displayLabel as elementText)
131133
guard var edgeList = adjacency[fromFingerprint],
132-
let idx = edgeList.lastIndex(where: { $0.elementText == elementText }) else {
134+
let idx = edgeList.lastIndex(where: { $0.displayLabel == elementText }) else {
133135
return
134136
}
135137

@@ -148,7 +150,7 @@ extension NavigationGraph {
148150

149151
// Keep the flat edges array in sync
150152
if let flatIdx = edges.lastIndex(where: {
151-
$0.fromFingerprint == fromFingerprint && $0.elementText == elementText
153+
$0.fromFingerprint == fromFingerprint && $0.displayLabel == elementText
152154
}) {
153155
edges[flatIdx] = edge
154156
}
@@ -159,17 +161,18 @@ extension NavigationGraph {
159161
lock.lock()
160162
defer { lock.unlock() }
161163
return adjacency[fromFingerprint]?
162-
.last(where: { $0.elementText == elementText })?.qValue ?? 1.0
164+
.last(where: { $0.displayLabel == elementText })?.qValue ?? 1.0
163165
}
164166

165167
// MARK: - Dead Edge Tracking
166168

167169
/// Mark an edge as dead (tap had no effect on the screen).
168170
/// Dead edges are excluded from future exploration plans.
171+
/// Uses displayLabel for consistency with Q-value lookups.
169172
///
170173
/// - Parameters:
171174
/// - fromFingerprint: The screen where the dead tap occurred.
172-
/// - elementText: The element text that was tapped.
175+
/// - elementText: The display label of the element that was tapped.
173176
func markEdgeDead(fromFingerprint: String, elementText: String) {
174177
lock.lock()
175178
defer { lock.unlock() }

Sources/mirroir-mcp/ScreenPlanner.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ enum ScreenPlanner {
9797
)
9898
return RankedElement(point: element.point, score: score, reason: reason)
9999
}
100-
.sorted { $0.point.tapY != $1.point.tapY ? $0.point.tapY < $1.point.tapY : $0.score > $1.score }
100+
.sorted { $0.score != $1.score ? $0.score > $1.score : $0.point.tapY < $1.point.tapY }
101101
}
102102

103103
// MARK: - Component-Based Plan Building
@@ -151,7 +151,7 @@ enum ScreenPlanner {
151151
return RankedElement(point: tapTarget, score: score, reason: reason,
152152
displayLabel: component.displayLabel, isBreadthNavigation: isBreadth)
153153
}
154-
.sorted { $0.point.tapY != $1.point.tapY ? $0.point.tapY < $1.point.tapY : $0.score > $1.score }
154+
.sorted { $0.score != $1.score ? $0.score > $1.score : $0.point.tapY < $1.point.tapY }
155155
}
156156

157157
// MARK: - Private
@@ -320,6 +320,6 @@ enum ScreenPlanner {
320320
isBreadthNavigation: element.isBreadthNavigation
321321
)
322322
}
323-
.sorted { $0.point.tapY != $1.point.tapY ? $0.point.tapY < $1.point.tapY : $0.score > $1.score }
323+
.sorted { $0.score != $1.score ? $0.score > $1.score : $0.point.tapY < $1.point.tapY }
324324
}
325325
}

0 commit comments

Comments
 (0)