Commit 74eab2a
authored
refactor: route selector-resolution structural stages into typed policy (#1744)
* refactor: route selector structural stages into typed policy
#1649 landed the per-caller ambiguity matrix and deliberately left four
structural columns out: occlusion, off-screen, hittable-ancestor promotion,
and the poll budget were per-caller pipeline code, so declaring them would
have been an unverifiable claim (nothing consumed them; flipping one left the
suite green).
This adds the missing half as a table with runners. `SELECTOR_PIPELINE_POLICIES`
(src/core/selector-pipeline-policy.ts) gives each caller ONE row naming its
ambiguity contract plus its four stages, and every stage is reached only
through a runner that reads the row:
- occlusion -> selectorPipelineCandidates (candidacy) and
resolveSelectorPipelineTarget (refusal). Acting rows exclude covered nodes
and refuse covered targets; `find` and the diagnosis probe keep them as
candidates and refuse at the target; reads and `wait` ignore them.
- promotion -> resolveSelectorPipelineTarget. The per-call-site
`promoteToHittableAncestor: boolean` is gone: click/press/longpress name
`promotedTarget`, fill/focus/scroll/drag endpoints and the native-ref
preflight name `resolvedTarget`. `find`'s below-the-root variant is a
declared value rather than a second local helper.
- off-screen -> throwIfOffscreenInteractionTarget, which now takes the row and
returns the node untouched (no iOS rescue round trip) for observation rows.
- poll -> selectorPollBudget, which createWaitPolling derives its deadline and
inter-poll delay from; the two wait loops carry a budget, every other row
carries none and cannot be polled.
Behavior is byte-identical. The acting refusal keeps its exact node, label and
details in every branch (promotion declines to retarget away from a covered
node, so the "both covered" case names the same node it always did), and
`find` carries the occlusion verdict to the focus/type seam rather than
raising it early, because find click/fill still delegate that refusal to the
interaction leaf's own error shape.
selector-pipeline-policy.test.ts drives EVERY row through EVERY runner,
including the rows whose answer is "skip" — the half that used to be an
absence of code, and an absence cannot fail. Each stage was proven red by
flipping its cell (occlusion, promotion, off-screen, poll, plus the
declare-only-what-is-enforced guard). The ADR 0011 occlusion/nonHittable
`via` pointers for the runtime tree paths now name the runner that makes the
decision, not the predicate it applies.
Closes #1656; prework for #1739 (waves 4-5).
* docs: state constraints instead of narrating the refactor
Comment pass over #1656: drop the "used to be per-caller code" /
"not module constants" / "rather than an omission" narration — a comment
should say what a future edit must respect, not what the previous shape was —
and compress the find occlusion-verdict and poll-budget notes to the
constraint they actually carry.
* refactor: make the selector pipeline the only door to the engine
Review of #1744: the structural rows were declared but bypassable. Read and
wait routes composed `selectorPipelineCandidates(row, nodes)` with the raw
`resolveSelectorChainWithPolicy(..., row.resolution)` and never entered the
promotion or off-screen stages, so flipping a read row's `promotion` or
`offscreen` changed only the policy unit tests — production `get`/`is`/`wait`
were unaffected, which is the unverifiable-column failure #1656 exists to
remove. Callers could also pair one row's candidate set with another row's
ambiguity contract, and `find list` reached the engine directly.
The owning interface (src/core/selector-pipeline.ts) now runs every stage a row
declares, skips included, and the stage functions are private to it:
- `resolveSelectorPipeline` — single-target rows: candidacy, ambiguity, the
replay-guard hook, promotion, occlusion, off-screen.
- `listSelectorPipelineMatches` — `reject-candidates` rows, returning the
candidate set AND the tree the row sees, so ranking and equivalence
classification judge the same nodes candidacy produced.
- `runNodePipelineStages` — the node stages for a target from a non-chain
matcher (`@ref`, find's fuzzy locator) or a narrowed candidate set.
A row whose off-screen stage refuses must supply a refusal shape, so flipping
an observation row to `refuse` fails on its real route instead of silently
observing. `find list` now names a `readList` row (the new
`reject-candidates`/no-rect ambiguity row) instead of calling the engine.
R17 selector-pipeline-ownership (scripts/layering/) makes the bypass
structurally inexpressible: only the owner may import the engine entry points.
Proven against a planted import in selector-read.ts, which the repo-wide scan
rejects with the entry points that replace it.
Flips now fail through REAL command routes, verified one at a time:
readUnique.occlusion/offscreen/promotion and wait.occlusion via get attrs / is
/ wait; readAny.offscreen via is exists and find; readList.occlusion via find
list; promotedTarget.promotion via runtime click. The wait route test needed an
advancing clock first — with the frozen one a refused wait spun instead of
failing, so the flip hung rather than asserting.
* refactor: drop find's dead candidate binding
The selector branch bound the row's candidate set and never read it: only the
acting classification needs that tree, and find's locator branch brings its own
matcher. Names what actually governs the locator target — the shared node
stages below, not a candidate set it never had.
* refactor: reserve the selector engine behind the pipeline owner
Review of #1744 (three blockers).
**Listing rows no longer claim stages they cannot run.** `find <q> list`
resolves to a candidate SET, so promotion, the off-screen guard and a poll
budget have nothing to apply to — a listing has no single element to retarget,
keep on screen, or wait for. `readList` now declares only the two stages a
listing executes (`SelectorListPolicy`: resolution + occlusion), and the
narrower shape is load-bearing: `runNodePipelineStages` and `selectorPollBudget`
take the full row, so handing them a listing row is a compile error rather than
a silently skipped stage. Pinned with `@ts-expect-error` — widening `readList`
makes the directives unused and fails the typecheck.
**The engine door is a specifier, not a symbol.** R17's regex could not see a
namespace import, a re-export, or a deferred `import()`, none of which mention
the symbol it matched. The two engine entries moved to
`@agent-device/selectors/engine`, and R19 enforces over the resolved import
graph, where every one of those forms is the same edge. Proven on the
repo-wide scan by planting each form into a shipped route: namespace import,
dynamic import, and `export *` laundering all come back red.
`resolveImportEdges` drops an edge whose specifier resolves to nothing, so a
specifier rule goes quiet — not red — if the subpath is ever retired. The gate
now says that out loud instead of scanning clean.
**R19, not R17.** #1750 allocates R17/R18. Verified free against origin/main
and that PR's diff, then validated by real merges in both directions: the
uniqueness gate passes either way and the three ids stay distinct.
The gate itself is new (`scripts/layering/rule-ids.ts`): two branches taking one
free number do not conflict in git, so nothing caught R17 twice. Matching whole
string literals is what separates a declaration from prose that names a rule,
and it is what let the gate see #1750's `const RULE = '…'` shape — the first
version missed it and would have been vacuous. `main`'s two pre-existing
collisions (R11, R13) are listed as known, not pinned by equality, so #1750
lands in either order without breaking this.
Also: the root façade now exposes no resolver at all, and its surface test
pins both doors.
* fix(layering): make each rule-id allowance expire with its collision
Review of #1744: `KNOWN_RULE_ID_COLLISIONS` filtered the exact R11/R13
collision strings, so once #1750 renames those rules apart the entries would
keep waving those very collisions through if anyone reintroduced them. "Inert"
was wrong — a stale allowance fails open, permanently.
`ruleIdCollisionFailures` now checks the transition from both sides: a
collision nobody allowed fails, AND an allowance whose collision is absent
from the scan fails as a stale allowance. The entry therefore has to be deleted
in the same change that removes the collision, and the list burns down to
empty, which admits nothing.
#1750 is still open, so the transitional entries stay for now (option (b)).
Verified against a scratch tree carrying that PR's rename: leaving the list
untouched reports both entries as stale; deleting them is clean; and
reintroducing `R11 names contracts-implementation-authority and
package-boundaries` afterwards is rejected. The last of those is also a unit
regression, so the post-transition guarantee is pinned rather than argued.1 parent 057ab1c commit 74eab2a
30 files changed
Lines changed: 1741 additions & 339 deletions
File tree
- packages
- contracts/src
- selectors
- src
- internal
- scripts/layering
- src
- commands/interaction/runtime
- __tests__
- test-utils
- core
- daemon/handlers
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
150 | 150 | | |
151 | 151 | | |
152 | 152 | | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
153 | 156 | | |
154 | 157 | | |
155 | | - | |
| 158 | + | |
156 | 159 | | |
157 | 160 | | |
158 | 161 | | |
| |||
171 | 174 | | |
172 | 175 | | |
173 | 176 | | |
| 177 | + | |
| 178 | + | |
174 | 179 | | |
175 | 180 | | |
176 | | - | |
| 181 | + | |
177 | 182 | | |
178 | 183 | | |
179 | 184 | | |
| |||
249 | 254 | | |
250 | 255 | | |
251 | 256 | | |
252 | | - | |
| 257 | + | |
253 | 258 | | |
254 | 259 | | |
255 | 260 | | |
| |||
370 | 375 | | |
371 | 376 | | |
372 | 377 | | |
373 | | - | |
| 378 | + | |
374 | 379 | | |
375 | 380 | | |
376 | 381 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
4 | 4 | | |
5 | 5 | | |
6 | 6 | | |
7 | | - | |
| 7 | + | |
8 | 8 | | |
9 | 9 | | |
10 | 10 | | |
| |||
18 | 18 | | |
19 | 19 | | |
20 | 20 | | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
21 | 25 | | |
22 | 26 | | |
23 | 27 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
| 2 | + | |
2 | 3 | | |
3 | 4 | | |
4 | 5 | | |
| 6 | + | |
5 | 7 | | |
6 | 8 | | |
7 | 9 | | |
8 | 10 | | |
9 | 11 | | |
10 | 12 | | |
11 | | - | |
12 | 13 | | |
13 | 14 | | |
14 | 15 | | |
| |||
201 | 202 | | |
202 | 203 | | |
203 | 204 | | |
204 | | - | |
| 205 | + | |
205 | 206 | | |
206 | | - | |
207 | | - | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
208 | 211 | | |
209 | | - | |
210 | | - | |
| 212 | + | |
| 213 | + | |
211 | 214 | | |
212 | 215 | | |
213 | | - | |
214 | | - | |
| 216 | + | |
| 217 | + | |
215 | 218 | | |
| 219 | + | |
| 220 | + | |
| 221 | + | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
| 231 | + | |
| 232 | + | |
| 233 | + | |
| 234 | + | |
| 235 | + | |
| 236 | + | |
216 | 237 | | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | | - | |
2 | 1 | | |
3 | | - | |
4 | | - | |
5 | | - | |
6 | | - | |
7 | | - | |
8 | | - | |
9 | 2 | | |
10 | 3 | | |
11 | 4 | | |
| |||
30 | 23 | | |
31 | 24 | | |
32 | 25 | | |
33 | | - | |
34 | | - | |
35 | | - | |
36 | | - | |
37 | | - | |
| 26 | + | |
38 | 27 | | |
39 | 28 | | |
40 | 29 | | |
| |||
59 | 48 | | |
60 | 49 | | |
61 | 50 | | |
| 51 | + | |
62 | 52 | | |
63 | 53 | | |
64 | 54 | | |
| |||
80 | 70 | | |
81 | 71 | | |
82 | 72 | | |
83 | | - | |
84 | 73 | | |
85 | 74 | | |
86 | 75 | | |
| |||
225 | 214 | | |
226 | 215 | | |
227 | 216 | | |
228 | | - | |
229 | | - | |
230 | | - | |
231 | | - | |
232 | | - | |
233 | | - | |
234 | | - | |
235 | | - | |
236 | | - | |
237 | | - | |
238 | 217 | | |
239 | 218 | | |
240 | | - | |
241 | | - | |
242 | | - | |
243 | | - | |
244 | | - | |
245 | | - | |
246 | | - | |
247 | | - | |
248 | | - | |
249 | | - | |
250 | | - | |
251 | | - | |
252 | | - | |
253 | | - | |
254 | | - | |
255 | | - | |
256 | | - | |
257 | | - | |
258 | | - | |
259 | | - | |
260 | | - | |
261 | | - | |
262 | | - | |
263 | | - | |
264 | | - | |
265 | | - | |
266 | | - | |
267 | | - | |
268 | | - | |
269 | | - | |
270 | | - | |
271 | | - | |
272 | | - | |
273 | | - | |
274 | | - | |
275 | | - | |
276 | | - | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
28 | 28 | | |
29 | 29 | | |
30 | 30 | | |
31 | | - | |
32 | | - | |
33 | | - | |
34 | | - | |
35 | | - | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
36 | 37 | | |
37 | 38 | | |
38 | 39 | | |
| |||
80 | 81 | | |
81 | 82 | | |
82 | 83 | | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
83 | 92 | | |
84 | 93 | | |
85 | 94 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
102 | 102 | | |
103 | 103 | | |
104 | 104 | | |
| 105 | + | |
105 | 106 | | |
106 | 107 | | |
107 | 108 | | |
| |||
670 | 671 | | |
671 | 672 | | |
672 | 673 | | |
| 674 | + | |
673 | 675 | | |
674 | 676 | | |
675 | 677 | | |
| |||
691 | 693 | | |
692 | 694 | | |
693 | 695 | | |
| 696 | + | |
| 697 | + | |
694 | 698 | | |
695 | 699 | | |
696 | 700 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
387 | 387 | | |
388 | 388 | | |
389 | 389 | | |
390 | | - | |
| 390 | + | |
391 | 391 | | |
392 | 392 | | |
393 | | - | |
| 393 | + | |
| 394 | + | |
| 395 | + | |
| 396 | + | |
394 | 397 | | |
395 | 398 | | |
396 | 399 | | |
| 400 | + | |
397 | 401 | | |
398 | 402 | | |
399 | 403 | | |
| |||
0 commit comments