Repository navigation
refactor(snapshot): type the acquisition producer beside the platform channel - #2000
Conversation
Size Report
npm unpacked components
Startup median (7 runs, lower is better):
Top changed chunks:
Top changed packed files
|
31fb491 to
50b7fd9
Compare
|
Not ready at |
|
Review of exact head The prior coverage and Fallow blockers are resolved cleanly: Limrun iOS and HarmonyOS now assert their real producers, and coverage/Fallow are green without suppressions. One remaining P2 contract gap blocks readiness: |
|
Re-reviewed |
Review follow-up on #2000: backend and producer were independent unions, so cross-channel pairs type-checked. SnapshotProvenance now owns the legal channel<->producer pairs; SnapshotBackend is its projection, SnapshotResult embeds the strict pair, SnapshotState embeds the optional-producer variant, and buildSnapshotState carries the pair through a narrowing helper so the fields never decorrelate. Negative type-level regression pins that invalid pairs cannot compile.
|
Re-reviewed |
Review follow-up on #2000: backend and producer were independent unions, so cross-channel pairs type-checked. SnapshotProvenance now owns the legal channel<->producer pairs; SnapshotBackend is its projection, SnapshotResult embeds the strict pair, SnapshotState embeds the optional-producer variant, and buildSnapshotState carries the pair through a narrowing helper so the fields never decorrelate. Negative type-level regression pins that invalid pairs cannot compile.
90cc71a to
c588c7b
Compare
|
Re-reviewed |
… channel Three producers with different guarantees share the backend: 'xctest' stamp (Apple runner, Appium page-source, limrun element trees), and the 'android' channel conflates the local uiautomator path with Appium trees the same way. Add SnapshotProducer as a required field on SnapshotResult so the compiler enumerates every producer, and carry it into SnapshotState. Types-before-semantics step for #1983; no consumer behavior changes.
c588c7b to
83fd0fe
Compare
Review follow-up on #2000: backend and producer were independent unions, so cross-channel pairs type-checked. SnapshotProvenance now owns the legal channel<->producer pairs; SnapshotBackend is its projection, SnapshotResult embeds the strict pair, SnapshotState embeds the optional-producer variant, and buildSnapshotState carries the pair through a narrowing helper so the fields never decorrelate. Negative type-level regression pins that invalid pairs cannot compile.
|
Implementation is sound and the prior typed-fixture failure is fixed; completed checks are green. One P2 remains: the new |
Review follow-up on #2000: backend and producer were independent unions, so cross-channel pairs type-checked. SnapshotProvenance now owns the legal channel<->producer pairs; SnapshotBackend is its projection, SnapshotResult embeds the strict pair, SnapshotState embeds the optional-producer variant, and buildSnapshotState carries the pair through a narrowing helper so the fields never decorrelate. Negative type-level regression pins that invalid pairs cannot compile.
83fd0fe to
30bd809
Compare
|
Re-review clean at |
|
…ad (#2008) * test(daemon): drive the timed-out wait decoration test on a fake clock The 1ms wait timeout raced the real clock: under CI load the first capture could take longer than 1ms to resolve, so runWithinWaitDeadline classified the poll capture-stalled/capture-truncated instead of a completed target-absent poll, and maybeWaitTimeoutSurfaceResponse skipped the "Current surface" decoration outright — causing the intermittent CI Coverage failure on PR #2000 (run 32752619090, head 30bd809). Reproduced the exact failure locally by pegging all CPUs during the test run, then fixed it the same way webdriver-interactor.test.ts's runFill does: vi.useFakeTimers() plus advanceTimersByTimeAsync so virtual time only moves when the test says so, removing the race entirely. Verified 10/10 passes under the same simulated load that reliably failed the old test. * test(daemon): simplify the fake-clock timed-out wait test per review Drop the justification comment and the settled-result wrapper: the capture path in this scenario is caught end-to-end by toDaemonResponse, so runWait cannot reject here, and the existing test name/assertions already state the invariant. --------- Co-authored-by: Claude <noreply@anthropic.com>
1 participant
Summary
Give every snapshot a typed producer — the acquisition component that made the raw tree — as a third identity axis beside the platform channel (
SnapshotBackend) and the in-plan capture strategy (SnapshotCaptureBackend).Today three producers with different guarantees share the
backend: 'xctest'stamp: the local Apple runner (whose output went through the runner's presentation — clip fold, effective geometry, scope), Appium page-source XML (provider-webdriver), and limrun element trees (provider-limrun) — the latter two reach the wire with none of those guarantees. The'android'channel conflates the local uiautomator path with Appium-sourced trees the same way. Daemon logic keyed on the channel cannot tell them apart.This is the types-before-semantics step for the shared-presentation track (#1983): no consumer changes behavior here. The field is what the host-side presenter wiring and the double-presentation removal will key on, and what a future XCTest-free simulator producer will register as.
SnapshotProducerunion + axis doc in@agent-device/kernel/snapshot;SnapshotState.producer(optional, daemon-side only — the value does not cross the CLI wire, same asbackend)SnapshotResult.produceris required, so the compiler enumerates every producer:apple-runner,android-uiautomator,appium-source,limrun-ios-tree,macos-helper,linux-atspi,agent-browser,harmonyos-uitestbuildSnapshotStatecarries the producer intoSnapshotState; the capture-handlerSnapshotDataseam passes it throughValidation
pnpm check:affected --run