Skip to content

Commit 9cc3309

Browse files
angusbezzinaclaude
andcommitted
feat(fidelity): anchored selectors, unified target rule, geometric card isolation
Fixes the two dogfooding failures in cli-got28: components could not be reliably isolated, and notes were associated with the wrong code. F1 — anchored, code-locating selectors: - SelectorEngine.generate anchors a non-identified target to its nearest identified ancestor and scopes indices within that component (#Settings.Models >> @save / text="…" / AXButton[n-in-scope]); component-local indices survive unrelated same-role tree changes. - Emit text= steps; add Selector.fromPath so a #<slash/path> that resolves to nothing is never emitted. F2 — deterministic component isolation: - One pure, unit-tested AnnotationTargetRule (deepest actionable, else deepest meaningful; never window/chrome/ghost) shared by all three sources (AX, NSView-tree, iOS UIView), replacing the macOS "deepest meaningful" vs iOS "nearest identified" split. A click inside a button binds to the button, not its glyph. - ComponentLadderSource + widen affordance to step selection up to the enclosing component. Docs (DECISIONS/README/spike/PARITY) corrected. F3 — deterministic note->code mapping: - AnnotationNote carries component / elementRole / elementText / unseeded, surfaced in markdown + JSON. Geometric card isolation (dogfood follow-up): - VirgilHUD's .axCardSurface seeds a card's id on a .background leaf that is a SIBLING of the content, not an ancestor, so pure-ancestry anchoring/widening could never reach the card. componentLadder now resolves enclosing components by frame containment (ancestor chain + each ancestor's direct children); the note component field and widening use it. The hover highlight shows a name tag so an element vs its enclosing card are distinguishable at a glance. 74 tests, incl. anchored-selector / mutation-stability / ambiguity / target-rule / widening / geometric-ladder / payload suites. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 7993a67 commit 9cc3309

19 files changed

Lines changed: 1050 additions & 122 deletions

DECISIONS.md

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,67 @@ Resolves the open decisions from the plan (planning/annotkit in the cli repo). P
99
| Repo location | Standalone repo at `~/Development/annotkit` | VirgilHUD consumes it during dev via a local SwiftPM path dependency; public remote created by the maintainer. |
1010
| Versioning | SemVer, 0.x pre-1.0 | Breaking changes allowed while 0.x; 1.0 marks a stable public API. |
1111
| Default element source | Accessibility hierarchy | The only strategy that surfaces SwiftUI `accessibilityIdentifier` values. |
12+
| Annotation target rule | Deepest actionable, else deepest meaningful; anchor the selector to the nearest identifier | One rule on both platforms. Supersedes the earlier macOS "deepest meaningful" and iOS "nearest identified" split. See below. |
1213
| Opt-in element source | View tree (NSView/UIView) | Surfaces concrete view class names; richer for AppKit/UIKit hosts. Collapses to hosting views in pure SwiftUI. |
1314
| `pathname` mapping | Host-supplied route, inferred fallback | A native app has no URL routes; the host sets a route, else infer from the key window title or identifier. |
1415
| Overlay coverage | Primary screen (MVP) | The overlay covers the primary display; SwiftUI-local points map to AX screen coordinates there. Full multi-display placement is deferred (cli-a99qm.4.2). |
1516
| Dev-only gating | `#if DEBUG` default + env override | On in DEBUG unless `ANNOTKIT_DISABLE`; off in release unless `ANNOTKIT_ENABLE`. Mirrors VirgilHUD `InspectMode`. |
1617
| MCP bridge in v1 | Deferred to F6 (optional) | The file and clipboard sinks cover the agent loop; the MCP/HTTP bridge is an optional later target, not part of the 1.0 critical path. |
1718
| Concurrency | Swift 6 language mode, strict | Public `Element`/`CapturedImage`/`AnnotationNote` are `Sendable`; `ElementSource`/`Annotation` are `@MainActor`. |
1819

20+
## Annotation target rule (cli-got28.2)
21+
22+
A click must isolate the component the user meant and produce a selector that
23+
locates its code. One rule, both platforms:
24+
25+
1. Hit-test to the deepest node at the point, then walk its ancestor chain.
26+
2. **Target = the deepest ACTIONABLE control** in the chain (button, link,
27+
checkbox, popup, slider, menu item, or anything exposing `AXPress`). A click
28+
anywhere inside a button binds to the button, not the static-text glyph that
29+
happens to be its deepest descendant.
30+
3. Else **target = the deepest MEANINGFUL element** — one carrying an identifier,
31+
a label, or a displayed value. A standalone `Text` inside a card is annotated
32+
in its own right (the card is not actionable, so it does not swallow the
33+
text); the selector engine then anchors it to the card's identifier.
34+
4. Never a target: the window, the application, window chrome (traffic lights),
35+
or a structural, unidentified, content-less group that spans (nearly) the
36+
whole window (an `NSHostingView` root `AXGroup` — the window in disguise).
37+
5. When the point hits nothing annotatable (decoration, dividers, padding beyond
38+
any frame), a `RegionAnchorSource` anchors the click to the nearest meaningful
39+
element as a REGION note rather than dropping it.
40+
41+
Why not "prefer the deepest *identified* ancestor"? Because seeding is partial: a
42+
button may be seeded but a standalone text inside a seeded card is usually not.
43+
Preferring the identified container would collapse every click inside a card onto
44+
the card and lose the specific element. Preferring the actionable/meaningful leaf
45+
and letting the SELECTOR anchor to the nearest identifier (`#Card >> text="…"`)
46+
keeps both the specificity and the code-locating anchor. Selection *widening*
47+
(cli-got28.2.3) exists for the times the user does want the enclosing component.
48+
49+
This replaces the earlier asymmetry (macOS "deepest meaningful", iOS "nearest
50+
identified") documented in `docs/spike-ax-pointquery.md`, now corrected. The pure
51+
decision lives in `AnnotationTargetRule` and is unit-tested independent of AX.
52+
53+
### Component containment is GEOMETRIC, not tree-ancestry (cli-got28.2)
54+
55+
"Which component encloses this element" is resolved by frame containment, not by
56+
walking the AX parent chain. A SwiftUI card seeded with `.axCardSurface` (the
57+
dominant VirgilHUD pattern) is a clear `Color.clear` background leaf carrying the
58+
identifier — and `.background` makes it a **sibling of the card's content, not an
59+
ancestor**. So the card's identifier never appears in the content's ancestor
60+
chain, and pure-ancestry anchoring/widening cannot reach it (it only worked for
61+
`.accessibilityElement(children: .contain)` containers, which *are* ancestors).
62+
63+
The macOS `componentLadder` therefore collects every identified element whose
64+
frame **encloses the point** and is larger than the target, smallest-first —
65+
scanning the ancestor chain plus each ancestor's direct children (where those
66+
background surfaces live), so it reaches sibling card surfaces without a full
67+
snapshot. This ladder drives selection widening and the note's `component` field.
68+
Selector *anchoring* (`#Card >> …`) still requires a true ancestor because the
69+
`>>` operator is descendant-based; when the component is a sibling surface the
70+
selector may be positional or text-based while the `component` field still names
71+
the card, so the note locates the right code either way.
72+
1973
## IP hygiene (carried into the F7 legal gate)
2074

2175
- Do not copy original Agentation source (PolyForm Shield 1.0.0, non-compete). Only the `AGENTATION_NOTES.md` file format is reused, reimplemented clean-room.

PARITY.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,9 @@ row; each asymmetry is closed by code or has a tracked mitigation.
1010
| Opt-in element source | NSView tree | (the default already walks the view tree) | macOS adds a view-tree opt-in alongside its AX default; iOS only needs the one source |
1111
| `accessibilityIdentifier` | via AX | via `UIView.accessibilityIdentifier` | both surface SwiftUI identifiers |
1212
| Concrete view class name | via NSView opt-in source | via the default UIView walk | none |
13-
| Hit test | `AXUIElementCopyElementAtPosition` + NSView `hitTest` | `UIView.hitTest(_:with:)` | iOS has no global AX point query; uses view hitTest. Tracked: F5.2 |
13+
| Hit test primitive | `AXUIElementCopyElementAtPosition` + NSView `hitTest` | `UIView.hitTest(_:with:)` | iOS has no global AX point query; uses view hitTest. Tracked: F5.2 |
14+
| Annotation target rule | shared `AnnotationTargetRule` over an AX candidate chain | shared `AnnotationTargetRule` over a UIView candidate chain | none — both build a `[TargetCandidate]` chain and apply the SAME rule (deepest actionable, else deepest meaningful). Closes the earlier split (macOS "deepest meaningful" vs iOS "nearest identified"), cli-got28.2 |
15+
| Component widening | `ComponentLadderSource` (AX chain) | `ComponentLadderSource` (UIView chain) | none — same ladder (target, then enclosing identified components) |
1416
| Coordinate space | Cocoa bottom-left to AX top-left flip | UIKit top-left native | iOS needs no flip; shared `ScreenSpace` used only on macOS |
1517
| Screenshot | ScreenCaptureKit / `cacheDisplay` | `UIGraphicsImageRenderer` + `drawHierarchy` | both capture own hierarchy only; no cross-window or secure overlays |
1618
| Overlay host | resizing `NSPanel` (toolbar corner idle, full screen annotating) | pass-through `UIWindow` | both interactive; selection via the shared SwiftUI catcher, not a global monitor |

README.md

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,13 @@ Annotation.install(sink: ClipboardSink(format: .json))
4040

4141
- **macOS** queries the app's own accessibility tree (the only strategy that
4242
surfaces SwiftUI `accessibilityIdentifier` values); **iOS** walks the UIView
43-
hierarchy. Both resolve a tap to the nearest identified element and generate a
44-
selector that round-trips a resolver.
43+
hierarchy. Both resolve a tap with the same rule: the **deepest actionable
44+
control** wins (a click inside a button binds to the button, not its label
45+
glyph), else the **deepest meaningful element** (a standalone text/label/value
46+
leaf is annotated in its own right). The generated selector then **anchors** a
47+
non-identified target to its nearest seeded `accessibilityIdentifier`
48+
(`#Settings.Models >> @Save`), so it round-trips a resolver and points an agent
49+
at the right component's code. See `DECISIONS.md`.
4550
- Notes are written in the `AGENTATION_NOTES.md` format that the
4651
`process-agentation-notes` skill consumes, or copied to the clipboard.
4752

Sources/AnnotKit/AnnotationSink.swift

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,23 @@ public struct AnnotationNote: Sendable, Hashable, Identifiable, Codable {
3535
public var selector: String
3636
/// The human-readable Element Path (ancestor chain).
3737
public var elementPath: String
38+
/// The seeded component the note is scoped to: the deepest
39+
/// `accessibilityIdentifier` in the target's ancestry (the target's own id
40+
/// when it is seeded, else its nearest seeded ancestor). This is the string an
41+
/// agent greps to find the component's Swift view; nil when nothing in the
42+
/// ancestry is seeded. Optional and additive — old files decode with nil.
43+
public var component: String?
44+
/// The bound element's accessibility role (`AXButton`, `AXStaticText`, …), so
45+
/// an agent knows what kind of view to look for. Optional/additive.
46+
public var elementRole: String?
47+
/// The bound element's displayed text/value, if any — a direct grep target
48+
/// (e.g. the button title or the `Text` string). Optional/additive.
49+
public var elementText: String?
50+
/// True when the bound element has NO `accessibilityIdentifier` of its own, so
51+
/// the selector had to anchor to an ancestor (``component``) or go positional.
52+
/// It marks a miss the user can turn into a seeding task rather than a silent
53+
/// misattribution. nil on notes captured before this field existed.
54+
public var unseeded: Bool?
3855
public var selectedText: String?
3956
public var comment: String
4057
public var screenshot: CapturedImage?
@@ -61,6 +78,7 @@ public struct AnnotationNote: Sendable, Hashable, Identifiable, Codable {
6178
/// unchanged.
6279
private enum CodingKeys: String, CodingKey {
6380
case id, route, selector, elementPath, selectedText, comment, screenshot, timestamp
81+
case component, elementRole, elementText, unseeded
6482
case regionOffset
6583
}
6684

@@ -69,6 +87,10 @@ public struct AnnotationNote: Sendable, Hashable, Identifiable, Codable {
6987
route: String? = nil,
7088
selector: String,
7189
elementPath: String,
90+
component: String? = nil,
91+
elementRole: String? = nil,
92+
elementText: String? = nil,
93+
unseeded: Bool? = nil,
7294
selectedText: String? = nil,
7395
comment: String,
7496
screenshot: CapturedImage? = nil,
@@ -80,6 +102,10 @@ public struct AnnotationNote: Sendable, Hashable, Identifiable, Codable {
80102
self.route = route
81103
self.selector = selector
82104
self.elementPath = elementPath
105+
self.component = component
106+
self.elementRole = elementRole
107+
self.elementText = elementText
108+
self.unseeded = unseeded
83109
self.selectedText = selectedText
84110
self.comment = comment
85111
self.screenshot = screenshot
Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
import Foundation
2+
3+
/// The annotation-relevant facts about one node in a hit's ancestor chain, read
4+
/// once by a platform adapter so the target rule stays pure — testable without
5+
/// the AX API or UIKit. Field names mirror ``SelectorMatchable`` where they
6+
/// overlap.
7+
public struct TargetCandidate: Sendable, Hashable {
8+
public let role: String
9+
public let identifier: String
10+
public let label: String
11+
public let value: String
12+
/// Exposes a press/edit action, or is one of the actionable roles.
13+
public let isActionable: Bool
14+
/// Window chrome (the traffic lights): a real, actionable button, but the
15+
/// WINDOW's chrome, not app content — its selector locates no app code, so it
16+
/// is never an annotation target.
17+
public let isChrome: Bool
18+
/// The window or application container. A background click must never resolve
19+
/// to the whole window/app.
20+
public let isContainerRoot: Bool
21+
/// A structural, unidentified, content-less group spanning (nearly) the whole
22+
/// window — an `NSHostingView` root `AXGroup`, the window in disguise.
23+
public let isWindowGhost: Bool
24+
25+
public init(
26+
role: String,
27+
identifier: String = "",
28+
label: String = "",
29+
value: String = "",
30+
isActionable: Bool = false,
31+
isChrome: Bool = false,
32+
isContainerRoot: Bool = false,
33+
isWindowGhost: Bool = false
34+
) {
35+
self.role = role
36+
self.identifier = identifier
37+
self.label = label
38+
self.value = value
39+
self.isActionable = isActionable
40+
self.isChrome = isChrome
41+
self.isContainerRoot = isContainerRoot
42+
self.isWindowGhost = isWindowGhost
43+
}
44+
45+
/// An actionable control eligible to be a target (not chrome, not the
46+
/// window/app root).
47+
var isEligibleActionable: Bool {
48+
isActionable && !isChrome && !isContainerRoot
49+
}
50+
51+
/// A meaningful element eligible to be a target: it carries an identifier, a
52+
/// label, a displayed value, or an action, and is not chrome, a container
53+
/// root, or a window-spanning ghost group.
54+
var isEligibleMeaningful: Bool {
55+
guard !isChrome, !isContainerRoot, !isWindowGhost else { return false }
56+
return !identifier.isEmpty || !label.isEmpty || !value.isEmpty || isActionable
57+
}
58+
}
59+
60+
/// The unified annotation target rule (see DECISIONS.md → "Annotation target
61+
/// rule"). Given the hit's ancestor chain, pick which node the annotation binds
62+
/// to. Pure and platform-independent: both the macOS AX adapter and the iOS
63+
/// view-tree adapter build a `[TargetCandidate]` chain and call ``targetIndex``,
64+
/// so they resolve a click identically and the rule is unit-tested on its own.
65+
public enum AnnotationTargetRule {
66+
/// Index in the root-first `chain` of the annotation target, or nil when
67+
/// nothing at the point is annotatable.
68+
///
69+
/// The deepest ACTIONABLE control wins — a click inside a button binds to the
70+
/// button, not the static-text glyph that is its deepest descendant. Failing
71+
/// that, the deepest MEANINGFUL element — a standalone text/label/value leaf
72+
/// is annotated in its own right, and the selector engine anchors it to its
73+
/// nearest identified ancestor.
74+
public static func targetIndex(in chain: [TargetCandidate]) -> Int? {
75+
for i in chain.indices.reversed() where chain[i].isEligibleActionable {
76+
return i
77+
}
78+
for i in chain.indices.reversed() where chain[i].isEligibleMeaningful {
79+
return i
80+
}
81+
return nil
82+
}
83+
84+
/// The component-widening ladder for `chain`: the target itself first, then
85+
/// each strictly-shallower identified ancestor, broadest last. Drives
86+
/// selection widening (a click-again steps up to the enclosing component).
87+
/// Empty when nothing is annotatable.
88+
public static func wideningLadder(in chain: [TargetCandidate]) -> [Int] {
89+
guard let target = targetIndex(in: chain) else { return [] }
90+
var ladder = [target]
91+
for i in stride(from: target - 1, through: 0, by: -1) {
92+
let c = chain[i]
93+
if c.isContainerRoot { break } // stop at the window; it is never a target
94+
if !c.identifier.isEmpty, !c.isWindowGhost { ladder.append(i) }
95+
}
96+
return ladder
97+
}
98+
}

Sources/AnnotKit/ElementSource.swift

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,14 @@ public protocol ElementSource {
1919
/// Snapshot every inspectable window and its element tree.
2020
func snapshot() -> [WindowSnapshot]
2121

22-
/// Resolve a screen point (AX top-left coordinates) to the deepest element
23-
/// under it, walked up to the nearest ancestor that carries a stable
24-
/// identity. Returns nil when nothing resolves.
22+
/// Resolve a screen point (AX top-left coordinates) to the annotation target
23+
/// under it, per the unified target rule (see DECISIONS.md): the deepest
24+
/// ACTIONABLE control at the point, else the deepest MEANINGFUL element
25+
/// (identifier / label / value / action), never the window, application,
26+
/// chrome, or a structural window-spanning group. Returns nil when nothing
27+
/// resolves (a `RegionAnchorSource` can then anchor the click to a nearby
28+
/// element instead of dropping it). The returned element's ``Element/path``
29+
/// carries its identified ancestors, which the selector engine anchors to.
2530
func hitTest(_ point: CGPoint) -> Element?
2631

2732
/// Generate the most-stable, round-tripping selector string for an element.
@@ -41,3 +46,15 @@ public protocol ElementSource {
4146
public protocol RegionAnchorSource {
4247
func regionAnchor(at point: CGPoint) -> Element?
4348
}
49+
50+
/// Optional element-source capability: the component-widening ladder at a point —
51+
/// the annotation target first (same element ``ElementSource/hitTest(_:)``
52+
/// returns), then each enclosing identified component, broadest last. Lets the
53+
/// session step a selection UP to a parent component for a coarser-grained note
54+
/// (a click-again / widen affordance while the composer is open) without
55+
/// re-hit-testing. Sources that cannot offer it simply don't conform; the session
56+
/// then disables widening.
57+
@MainActor
58+
public protocol ComponentLadderSource {
59+
func componentLadder(at point: CGPoint) -> [Element]
60+
}

0 commit comments

Comments
 (0)