refactor(daemon): one capture-input builder and one admit-then-bind step - #1876
Conversation
Size Report
Startup median (7 runs, lower is better):
Top changed chunks: no changes in the largest emitted chunks. |
|
Reviewed exact head |
5029e01 to
30435df
Compare
thymikee
changed the title
|
Re-scoped since your review at Both blockers from that review are addressed by removing the thing they applied to. This PR no longer migrates
What is left is behavior-neutral and consumed: one capture-input builder ( The selector seam that was here has moved to #1877, its first consuming command unit. ADR 0019 line 686 prohibits a facet PR with no consuming command, and line 448 permits an early substrate only when it is dead-code clean — with Two honesty notes carried in the body rather than smoothed over: both extractions have a single consumer today, so they are named steps rather than deduplication — the second caller arrives with #1877 — and All 28 required checks green at this head. |
|
Reviewed exact head |
…nsumer Takes ownership of the request-bound selector capture seam from #1876, which cannot ship standalone: with find's cutover deferred it had no consuming command (ADR 0019 §10) and was not dead-code clean (check:production-exports 19 -> 20). `get` is its first consumer, so it lands here. Adopts find's handoff as given. The one shape change, approved by the coordinator: the selector family gets its own capture uses carrying a PREFERRED `readTextAtPoint`, declared ALONGSIDE the snapshot uses so `snapshot`/`diff` keep binding exactly what they bind today. The read is surfaced through the existing arms of `bindSnapshotCaptureRuntime`, reusing the same selectActiveAppSnapshot / selectSnapshotWithoutActiveApp selectors — no second plan-to-operation dispatch. `get` now runs through `createBoundSelectorRuntime`; `resolveBoundGetRuntime` and its test are deleted as superseded, and `'get'` leaves the `createSelectorRuntime` capability union. The legacy read adapter survives for `find <q> get text` and is selected by which command constructed the runtime — never by failure, family, environment, or flag — so `get` cannot reach it. It retires in find's cutover, where the last consumer moves.
Behaviour-neutral. No descriptor changes platform execution, the cutover table is untouched, and no contract surface is added. - buildRuntimeCaptureInput moves to its own module so every request-bound capture consumer builds CaptureSnapshotInput one way. - The admit-then-bind sequence in the snapshot/diff resolver becomes one named step, ready for the selector units' second caller. - handlers/find.ts splits into focused target-capture and match-resolution concepts (600 -> 346 lines); behaviour unchanged.
30435df to
3bf786f
Compare
|
Reviewed exact head |
thymikee
added
the
ready-for-human
…nsumer Takes ownership of the request-bound selector capture seam from #1876, which cannot ship standalone: with find's cutover deferred it had no consuming command (ADR 0019 §10) and was not dead-code clean (check:production-exports 19 -> 20). `get` is its first consumer, so it lands here. Adopts find's handoff as given. The one shape change, approved by the coordinator: the selector family gets its own capture uses carrying a PREFERRED `readTextAtPoint`, declared ALONGSIDE the snapshot uses so `snapshot`/`diff` keep binding exactly what they bind today. The read is surfaced through the existing arms of `bindSnapshotCaptureRuntime`, reusing the same selectActiveAppSnapshot / selectSnapshotWithoutActiveApp selectors — no second plan-to-operation dispatch. `get` now runs through `createBoundSelectorRuntime`; `resolveBoundGetRuntime` and its test are deleted as superseded, and `'get'` leaves the `createSelectorRuntime` capability union. The legacy read adapter survives for `find <q> get text` and is selected by which command constructed the runtime — never by failure, family, environment, or flag — so `get` cannot reach it. It retires in find's cutover, where the last consumer moves.
Two edits, per find's ADDENDUM.md: 1. `includeRects` returns to `buildRuntimeCaptureInput`. It was removed from #1876 as unconsumed; the selector capture path is genuinely its first consumer (a Web rect capture requests bounds explicitly), so it lands here under the same rule that moved the seam. `snapshot`/`diff` pass nothing. 2. The per-capture `signal` is dropped, not restored. `CaptureSnapshotInput` has no such field on this stack — it moved to `wait` (#1875) with the regression that proves per-poll abort and quiescence. `get` captures once per resolution and never polls, so nothing here needs it. The seam test and fixture coverage for it moves with the contract rather than being kept against a field that no longer exists.
|
…nsumer Takes ownership of the request-bound selector capture seam from #1876, which cannot ship standalone: with find's cutover deferred it had no consuming command (ADR 0019 §10) and was not dead-code clean (check:production-exports 19 -> 20). `get` is its first consumer, so it lands here. Adopts find's handoff as given. The one shape change, approved by the coordinator: the selector family gets its own capture uses carrying a PREFERRED `readTextAtPoint`, declared ALONGSIDE the snapshot uses so `snapshot`/`diff` keep binding exactly what they bind today. The read is surfaced through the existing arms of `bindSnapshotCaptureRuntime`, reusing the same selectActiveAppSnapshot / selectSnapshotWithoutActiveApp selectors — no second plan-to-operation dispatch. `get` now runs through `createBoundSelectorRuntime`; `resolveBoundGetRuntime` and its test are deleted as superseded, and `'get'` leaves the `createSelectorRuntime` capability union. The legacy read adapter survives for `find <q> get text` and is selected by which command constructed the runtime — never by failure, family, environment, or flag — so `get` cannot reach it. It retires in find's cutover, where the last consumer moves.
Two edits, per find's ADDENDUM.md: 1. `includeRects` returns to `buildRuntimeCaptureInput`. It was removed from #1876 as unconsumed; the selector capture path is genuinely its first consumer (a Web rect capture requests bounds explicitly), so it lands here under the same rule that moved the seam. `snapshot`/`diff` pass nothing. 2. The per-capture `signal` is dropped, not restored. `CaptureSnapshotInput` has no such field on this stack — it moved to `wait` (#1875) with the regression that proves per-poll abort and quiescence. `get` captures once per resolution and never polls, so nothing here needs it. The seam test and fixture coverage for it moves with the contract rather than being kept against a field that no longer exists.
…nsumer Takes ownership of the request-bound selector capture seam from #1876, which cannot ship standalone: with find's cutover deferred it had no consuming command (ADR 0019 §10) and was not dead-code clean (check:production-exports 19 -> 20). `get` is its first consumer, so it lands here. Adopts find's handoff as given. The one shape change, approved by the coordinator: the selector family gets its own capture uses carrying a PREFERRED `readTextAtPoint`, declared ALONGSIDE the snapshot uses so `snapshot`/`diff` keep binding exactly what they bind today. The read is surfaced through the existing arms of `bindSnapshotCaptureRuntime`, reusing the same selectActiveAppSnapshot / selectSnapshotWithoutActiveApp selectors — no second plan-to-operation dispatch. `get` now runs through `createBoundSelectorRuntime`; `resolveBoundGetRuntime` and its test are deleted as superseded, and `'get'` leaves the `createSelectorRuntime` capability union. The legacy read adapter survives for `find <q> get text` and is selected by which command constructed the runtime — never by failure, family, environment, or flag — so `get` cannot reach it. It retires in find's cutover, where the last consumer moves.
Two edits, per find's ADDENDUM.md: 1. `includeRects` returns to `buildRuntimeCaptureInput`. It was removed from #1876 as unconsumed; the selector capture path is genuinely its first consumer (a Web rect capture requests bounds explicitly), so it lands here under the same rule that moved the seam. `snapshot`/`diff` pass nothing. 2. The per-capture `signal` is dropped, not restored. `CaptureSnapshotInput` has no such field on this stack — it moved to `wait` (#1875) with the regression that proves per-poll abort and quiescence. `get` captures once per resolution and never polls, so nothing here needs it. The seam test and fixture coverage for it moves with the contract rather than being kept against a field that no longer exists.
…nsumer Takes ownership of the request-bound selector capture seam from #1876, which cannot ship standalone: with find's cutover deferred it had no consuming command (ADR 0019 §10) and was not dead-code clean (check:production-exports 19 -> 20). `get` is its first consumer, so it lands here. Adopts find's handoff as given. The one shape change, approved by the coordinator: the selector family gets its own capture uses carrying a PREFERRED `readTextAtPoint`, declared ALONGSIDE the snapshot uses so `snapshot`/`diff` keep binding exactly what they bind today. The read is surfaced through the existing arms of `bindSnapshotCaptureRuntime`, reusing the same selectActiveAppSnapshot / selectSnapshotWithoutActiveApp selectors — no second plan-to-operation dispatch. `get` now runs through `createBoundSelectorRuntime`; `resolveBoundGetRuntime` and its test are deleted as superseded, and `'get'` leaves the `createSelectorRuntime` capability union. The legacy read adapter survives for `find <q> get text` and is selected by which command constructed the runtime — never by failure, family, environment, or flag — so `get` cannot reach it. It retires in find's cutover, where the last consumer moves.
Two edits, per find's ADDENDUM.md: 1. `includeRects` returns to `buildRuntimeCaptureInput`. It was removed from #1876 as unconsumed; the selector capture path is genuinely its first consumer (a Web rect capture requests bounds explicitly), so it lands here under the same rule that moved the seam. `snapshot`/`diff` pass nothing. 2. The per-capture `signal` is dropped, not restored. `CaptureSnapshotInput` has no such field on this stack — it moved to `wait` (#1875) with the regression that proves per-poll abort and quiescence. `get` captures once per resolution and never polls, so nothing here needs it. The seam test and fixture coverage for it moves with the contract rather than being kept against a field that no longer exists.
…nsumer Takes ownership of the request-bound selector capture seam from #1876, which cannot ship standalone: with find's cutover deferred it had no consuming command (ADR 0019 §10) and was not dead-code clean (check:production-exports 19 -> 20). `get` is its first consumer, so it lands here. Adopts find's handoff as given. The one shape change, approved by the coordinator: the selector family gets its own capture uses carrying a PREFERRED `readTextAtPoint`, declared ALONGSIDE the snapshot uses so `snapshot`/`diff` keep binding exactly what they bind today. The read is surfaced through the existing arms of `bindSnapshotCaptureRuntime`, reusing the same selectActiveAppSnapshot / selectSnapshotWithoutActiveApp selectors — no second plan-to-operation dispatch. `get` now runs through `createBoundSelectorRuntime`; `resolveBoundGetRuntime` and its test are deleted as superseded, and `'get'` leaves the `createSelectorRuntime` capability union. The legacy read adapter survives for `find <q> get text` and is selected by which command constructed the runtime — never by failure, family, environment, or flag — so `get` cannot reach it. It retires in find's cutover, where the last consumer moves.
Two edits, per find's ADDENDUM.md: 1. `includeRects` returns to `buildRuntimeCaptureInput`. It was removed from #1876 as unconsumed; the selector capture path is genuinely its first consumer (a Web rect capture requests bounds explicitly), so it lands here under the same rule that moved the seam. `snapshot`/`diff` pass nothing. 2. The per-capture `signal` is dropped, not restored. `CaptureSnapshotInput` has no such field on this stack — it moved to `wait` (#1875) with the regression that proves per-poll abort and quiescence. `get` captures once per resolution and never polls, so nothing here needs it. The seam test and fixture coverage for it moves with the contract rather than being kept against a field that no longer exists.
1 participant
Summary
Behaviour-neutral cleanup around the request-bound snapshot capture path. No descriptor changes its platform execution, no contract surface is added, and no workspace package is touched — the diff is five source files plus one test-helper import path. Three things:
buildRuntimeCaptureInputmoves out ofsnapshot-runtime-binding.tsintosnapshot-runtime-capture-input.ts— the one place a daemon request becomes neutralCaptureSnapshotInput. Its parameter list is exactly whatsnapshot/diffpass; nothing speculative.admitAndBindSnapshotCapture, module-private. Same behaviour, one place.handlers/find.tssplits, 600 → 346 lines. It was already past the 300-line tripwire and answered three questions at once. The target-capture policy (interactive-only tree plus find's two sparse-recovery re-captures) and the match-resolution pipeline (locator/selector matching, on-screen preference, interactive ranking, the ambiguous-match error) move to focused modules. Pure refactor: identical behaviour, response shapes, capture path, and tests.Scope note
This PR was re-scoped twice under review, each time correctly:
It began as the
findcutover. Declaringfinddevice-runtimeclaims its whole platform-execution projection (ADR 0019 §6), whilefind focusandfind typestill reach the device throughdispatchCommand— Wave 5 surfaces — and a third edge of the same class,find <q> get text→dispatchCommand(device, 'read'), is shared withget.findkeepsLEGACY_PLATFORM_EXECUTION, its capability bucket, itsrequireCommandSupported('find', …)admission, and itsHARMONYOS_SUPPORTED_COMMANDS/WEB_QUERY_COMMANDSentries. Rule id R35 is withdrawn from the cutover table entirely rather than left half-claimed, and stays reserved for find's real cutover.It then carried the selector capture seam, and then
CaptureSnapshotInput.signalwith its exportedcaptureSnapshotSignalhelper and Apple/Linux composition. Both are now gone, removed for the same reason: ADR 0019 §10 prohibits a facet PR with no consuming command, and §6 admits an early behaviour-neutral substrate only when it is dead-code clean.On the signal contract specifically: an earlier revision of this body carried it with a caveat that no caller set the field. Review escalated that caveat to a blocker, and rightly — an unset contract field means production exercises only the
undefinedbranch, and this PR could not carry a regression proving the per-poll abort and quiescence behaviour it claimed. The field, the helper, and the Apple/Linux composition sites now land in refactor: migrate wait to request-bound runtime #1875 (wait), whoserunWithinWaitDeadlineis the route that actually sets it and which owns the regression proving the deadline aborts an in-flight capture, waits for it to quiesce, and producescapture-stalledrather thantarget-absent. The selector capture seam went to refactor: migrate get to the request-bound device runtime #1877 (get) on the same principle. Both were handed over as reviewed patches with design notes.What remains has live production consumers today and is dead-code clean:
check:production-exportsis unchanged frommain.Validation
pnpm check:affected --runfrom a clean committed tree. Behaviour neutrality is carried by the existing suites rather than new assertions —snapshot,diff, andfindkeep their existing paths, and every touched command's test file is byte-identical tomain, so they exercise the splitfindroute unchanged.pnpm check:layeringis green with the migrated-command list identical tomain.Tradeoffs and known gaps
get.admitAndBindSnapshotCaptureis deliberately module-private so this PR does not grow the export surface;get's patch flips it to exported, a one-word change.find.tsthree-way split (module headers, imports, exports — the unavoidable cost of a split the tripwire required) and +2,144 B the two extractions, of which roughly 1.4 kB is the capture-input builder moving rather than new code. Packaged deltas: see the CI Size report on this head rather than a local comparison.website/docs/**orskills/**was touched.Touched files: 6. Scope narrowed three times; it never expanded.