Skip to content

Commit 48e3efe

Browse files
committed
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.
1 parent 7f55578 commit 48e3efe

8 files changed

Lines changed: 39 additions & 51 deletions

File tree

packages/selectors/src/internal/resolution-policy.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,8 @@ import type { SelectorResolutionOptions } from './public-resolution-types.ts';
3232
* companion structural table, `src/core/selector-pipeline-policy.ts` (#1656),
3333
* whose rows each name one row of this matrix. They live there rather than
3434
* here because this package is deliberately blind to snapshot occlusion
35-
* annotations, backend visibility probes, and the wait clock; a column here
36-
* would be a claim nothing in this package could enforce, which is exactly the
37-
* unverifiable-column failure the #1649 review caught.
35+
* annotations, backend visibility probes, and the wait clock: a column here
36+
* would be a claim nothing in this package could enforce (#1649 review).
3837
*/
3938

4039
export type KnobBackedSelectorAmbiguity = 'disambiguate' | 'fail-closed' | 'first-match';

src/commands/interaction/runtime/__tests__/resolution-policy-parity.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -166,9 +166,9 @@ test('rect-requiring rows skip rectless nodes; read and wait rows accept them',
166166
* The matrix may only declare what it can enforce (#1649 review). An earlier
167167
* revision carried occlusion / off-screen / promotion / poll columns that no
168168
* code consumed, so changing them left both behavior and the suite green —
169-
* an unverifiable claim reading as truth. Those four stages now live in the
169+
* an unverifiable claim reading as truth. Those four stages belong to the
170170
* structural table (`src/core/selector-pipeline-policy.ts`, #1656), where
171-
* runners consume them; this still fails if such a field reappears HERE, where
171+
* runners consume them; this still fails if such a field appears HERE, where
172172
* the selectors package has nothing to enforce it with.
173173
*/
174174
test('policy rows declare only the fields this matrix actually enforces', () => {

src/commands/interaction/runtime/resolution.test.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -603,9 +603,10 @@ test('throwIfOffscreenInteractionTarget: off-screen + backend confirms -> return
603603
});
604604

605605
test('throwIfOffscreenInteractionTarget: a row whose off-screen stage is ignore returns the node untouched', async () => {
606-
// #1656: reads and waits answer about the tree as captured, so the same
607-
// node the acting rows refuse below passes through here — and the iOS
608-
// rescue hook is never consulted, which is why it would throw if it were.
606+
// #1656: reads and waits answer about the tree as captured, so the node the
607+
// acting rows refuse below passes through here. The rescue hook throws on
608+
// contact, so a row that started refusing would fail loudly rather than
609+
// quietly spending an iOS round trip.
609610
const device = createInteractionDevice(makeSnapshotState([]), {
610611
confirmOffscreenTargetVisible: async () => {
611612
throw new Error('an observation row must not spend the rescue round trip');

src/commands/interaction/runtime/resolution.ts

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -142,9 +142,9 @@ type ResolveInteractionTargetParams = {
142142
requireInteractive: boolean;
143143
/**
144144
* The structural pipeline this action runs (#1656): occlusion, off-screen,
145-
* and hittable-ancestor promotion are the row's decisions, not per-call-site
146-
* booleans. `promotedTarget` for tap-shaped actions, `resolvedTarget` for the
147-
* actions that must keep the element they resolved.
145+
* and hittable-ancestor promotion are the row's decisions. `promotedTarget`
146+
* for tap-shaped actions, `resolvedTarget` for the actions that must keep
147+
* the element they resolved.
148148
*/
149149
pipeline: ActingPipelinePolicy;
150150
/**
@@ -937,9 +937,8 @@ export async function preflightNativeRefInteraction(
937937
fallbackLabel: target.fallbackLabel ?? '',
938938
});
939939
if (!resolved) return {};
940-
// `resolvedTarget`, whatever the command: this row's `none` promotion is what
941-
// makes "the preflight never changes which element the backend acts on" a
942-
// declared property rather than an omission at this call site.
940+
// `resolvedTarget` whatever the command: its `none` promotion is what holds
941+
// ADR 0011's "the preflight never changes which element the backend acts on".
943942
const pipeline = SELECTOR_PIPELINE_POLICIES.resolvedTarget;
944943
const node = resolveInteractionPipelineTarget(pipeline, nodes, resolved.node, {
945944
action,

src/commands/interaction/runtime/wait-polling.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,9 +49,8 @@ type WaitFailurePolling = {
4949

5050
/**
5151
* The poll stage of the caller's selector-pipeline row (#1656): the deadline
52-
* and the inter-poll delay come from the row, not from module constants, so a
53-
* caller cannot invent a budget and a row that resolves against a single
54-
* capture cannot be polled at all.
52+
* and the inter-poll delay come from the row, so a caller cannot invent a
53+
* budget and a row that resolves against a single capture cannot be polled.
5554
*/
5655
export function createWaitPolling(
5756
runtime: WaitPollingRuntime,

src/core/selector-pipeline-policy.test.ts

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,9 @@ import {
1313
} from './selector-pipeline-policy.ts';
1414

1515
/**
16-
* #1656: the structural stages are routed through the runners below, so this
17-
* file is what makes the table's cells load-bearing. Every row is driven
18-
* through every runner — including the rows whose answer is "skip this stage",
19-
* which is the half that used to be invisible: `is` not consulting occlusion
20-
* was an absence of code, and an absence cannot fail. Flip any cell in the
21-
* table and an assertion here changes.
16+
* #1656: every row is driven through every runner — including the rows whose
17+
* answer is "skip this stage", since a skip nothing exercises is a claim that
18+
* cannot fail. Flip any cell in the table and an assertion here changes.
2219
*
2320
* Which stages a given CALLER runs is a separate question, pinned by the
2421
* caller-level suites (resolution.test.ts, find handler tests, selector-read

src/core/selector-pipeline-policy.ts

Lines changed: 16 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -16,18 +16,14 @@ import {
1616
* A caller names ONE row here and gets its whole pipeline shape: which
1717
* ambiguity contract resolves the selector (`resolution`, the matrix row), and
1818
* which structural stages run around that resolution — occlusion, off-screen,
19-
* hittable-ancestor promotion, and the poll budget. Those four stages used to
20-
* be per-caller pipeline code, so "`is` never consults occlusion" was true only
21-
* as an ABSENCE of code: nothing declared it and nothing could fail if a later
22-
* edit added it. Here it is a value, and the stage runners below are the only
23-
* door to each stage, so the value decides.
19+
* hittable-ancestor promotion, and the poll budget. The runners below are the
20+
* only door to each stage, so a row that skips one still says so as a value:
21+
* making `is` consult occlusion means editing this table, not adding a call.
2422
*
25-
* Rule of this table, inherited from the #1649 review: a row may only declare
26-
* what these runners enforce. Every field is read by the runner named in its
27-
* doc comment, and every row — including the ones whose stages are skips — is
28-
* driven through those runners in selector-pipeline-policy.test.ts, so flipping
29-
* any cell changes an assertion. Do not add a column until its runner exists;
30-
* an unconsumed column reads as truth while being free to drift.
23+
* A row may only declare what these runners enforce (#1649 review). Every row,
24+
* skips included, is driven through every runner in
25+
* selector-pipeline-policy.test.ts, so flipping any cell changes an assertion —
26+
* add a column only once a runner reads it.
3127
*/
3228

3329
/**
@@ -83,9 +79,9 @@ export type SelectorPollBudget = {
8379

8480
/**
8581
* Consumed by `selectorPollBudget`, which `createWaitPolling` derives its
86-
* deadline and sleep from. `'none'` is a real answer, not an omission: a row
87-
* that resolves against one capture has no polling contract, and asking for
88-
* its budget is a bug rather than a defaulting opportunity.
82+
* deadline and sleep from. `'none'` is an answer, not an omission: a row that
83+
* resolves against one capture has no polling contract, so asking for its
84+
* budget is a caller bug — never a place to default one in.
8985
*/
9086
export type SelectorPollStage = SelectorPollBudget | 'none';
9187

@@ -203,13 +199,13 @@ export function selectorPipelineCandidates(
203199

204200
/**
205201
* Stage 2, post-resolution: promotion, then the occlusion verdict on what
206-
* promotion produced — the order every acting caller ran by hand.
202+
* promotion produced.
207203
*
208-
* `occluded` carries the node to NAME in the refusal, which is the promotion
209-
* input when the match itself is covered (promotion declines to retarget away
210-
* from a covered node) and the promotion output otherwise. The refusal shape
211-
* stays with the caller: the same verdict is a thrown interaction error, a
212-
* daemon response, or a diagnosis, and those are not interchangeable.
204+
* `occluded` carries the node to NAME in the refusalthe promotion input
205+
* when the match itself is covered (promotion declines to retarget away from a
206+
* covered node), the promotion output otherwise. The refusal SHAPE stays with
207+
* the caller: this verdict becomes a thrown interaction error, a daemon
208+
* response, or a diagnosis, and those are not interchangeable.
213209
*/
214210
export type SelectorPipelineTarget =
215211
| { kind: 'target'; node: SnapshotNode }

src/daemon/handlers/find.ts

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -77,11 +77,9 @@ type ResolvedMatch = {
7777
nodes: SnapshotState['nodes'];
7878
actionFlags: Record<string, unknown>;
7979
/**
80-
* The occlusion verdict the `findAct` row produced for this match, carried
81-
* rather than raised: `find click`/`fill` re-enter the interaction leaf,
82-
* which refuses covered targets in the acting row's own error shape, so only
83-
* the focus/type seam — which dispatches to the device directly — surfaces
84-
* it here.
80+
* Set when find's row refuses this match as covered. Only the focus/type
81+
* seam surfaces it: click/fill re-enter the interaction leaf, which owns
82+
* that refusal's shape and raises it against the same node.
8583
*/
8684
occludedNode?: SnapshotState['nodes'][number];
8785
};
@@ -288,9 +286,8 @@ function resolveFindMatch(params: {
288286
}): FindMatchResult {
289287
const { nodes, locator, query, selectorExpression, flags, platform } = params;
290288
const pipeline = SELECTOR_PIPELINE_POLICIES.findAct;
291-
// The occlusion stage of find's row: it declares `refuse`, so covered nodes
292-
// stay candidates here (find ranks and reports them) and are refused later,
293-
// at the target. Acting rows exclude them from candidacy instead.
289+
// The occlusion stage of find's row: `refuse` keeps covered nodes as
290+
// candidates here — find ranks and reports them — and refuses at the target.
294291
const searchableNodes = selectorPipelineCandidates(
295292
pipeline,
296293
nodes.filter((node) => !isRootInteractionContainer(node, nodes[0])),

0 commit comments

Comments
 (0)