test(ios-snapshot): cover runner-presented interactive pipeline - #3009
Conversation
Size Report
Startup median (7 runs, lower is better):
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
6 issues found across 8 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/capture-kit/src/ios-snapshot-engine/conformance-harness.ts">
<violation number="1" location="packages/capture-kit/src/ios-snapshot-engine/conformance-harness.ts:213">
P3: The failure arm of `runnerPresentationAgrees` returns after comparing Swift's error to the acquired TypeScript error, before `runRunnerComposition` ever runs. For a runner-presented corpus that fails identically on both languages, the `stage: 'presented'` adapter — the pipeline this PR is meant to exercise — is never invoked, so an acquired/presented failure-code mismatch (e.g., the presented adapter throwing `projection-mismatch` while the acquired adapter reports the same typed error) passes silently. Run the presented composition in this arm too and assert its typed error agrees, instead of returning on the error comparison alone.</violation>
<violation number="2" location="packages/capture-kit/src/ios-snapshot-engine/conformance-harness.ts:247">
P3: `runRunnerComposition` presents the payload once via `presentIosSnapshot` and then calls `publishIosSnapshot`, which re-runs `presentIosSnapshot` internally on the same input and request (engine.ts line 31). Every runner-presented case therefore compacts and validates the full payload twice — per case, per fuzz seed. This doubles work that this test path controls directly; consider deriving the published payload from the single presentation, or documenting the double-present as intentional.</violation>
<violation number="3" location="packages/capture-kit/src/ios-snapshot-engine/conformance-harness.ts:300">
P3: `qualityMembershipAgrees` short-circuits whenever `testCase.scope === null`, so unscoped runner-presented cases never assert that the presented pipeline omits quality nodes. If Swift's presenter or the host `stage: 'presented'` adapter ever started emitting quality evidence for an unscoped corpus, this differential would not flag it (the qualityPayload would be validated for shape but never checked for emptiness). Compare against the empty expected membership instead of skipping the check.</violation>
<violation number="4" location="packages/capture-kit/src/ios-snapshot-engine/conformance-harness.ts:357">
P2: This comparator drops `identifier` and `value` from semantic membership, so the runner differential can miss lost selector identity or entirely lose unlabeled semantic controls. Preserve the same semantic fields used by the projection and existing property checks when building the comparison identity.</violation>
</file>
<file name="scripts/ios-snapshot-differential.test.ts">
<violation number="1" location="scripts/ios-snapshot-differential.test.ts:75">
P3: The exclusion list in the `RUNNER_UNSUPPORTED` assertion (lines 75-76) and the inclusion list in the runner-presented filter (lines 89-90) are the same two case names written twice. Nothing ties them together: adding a future non-Swift case to only the exclusion list passes the `deepEqual` against `Object.keys(RUNNER_UNSUPPORTED)` while silently dropping that case from runner-presented coverage. Extract the pair into one shared constant (e.g. `RUNNER_PRESENTED_NON_SWIFT_CASES`) and reference it from both filters.</violation>
<violation number="2" location="scripts/ios-snapshot-differential.test.ts:80">
P3: `assert.deepEqual` compares these arrays positionally, so the completeness guard silently depends on ordering: `Object.keys(RUNNER_UNSUPPORTED)` follows declaration order while `fixture.cases` follows the JSON file order. Inserting a new omitted case ahead of an existing one in the corpus, or reordering the declared keys, fails the guard even when the declared set is identical and complete, with a message that claims a missing asymmetry. Compare both sides as sorted sets so the contract is order-agnostic.</violation>
</file>
Tip: instead of fixing issues one by one fix them all with cubic
Re-trigger cubic
| ...new Set( | ||
| nodes | ||
| .filter((node) => node.label !== null) | ||
| .map((node) => JSON.stringify([node.type, node.label])), |
There was a problem hiding this comment.
P2: This comparator drops identifier and value from semantic membership, so the runner differential can miss lost selector identity or entirely lose unlabeled semantic controls. Preserve the same semantic fields used by the projection and existing property checks when building the comparison identity.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/capture-kit/src/ios-snapshot-engine/conformance-harness.ts, line 357:
<comment>This comparator drops `identifier` and `value` from semantic membership, so the runner differential can miss lost selector identity or entirely lose unlabeled semantic controls. Preserve the same semantic fields used by the projection and existing property checks when building the comparison identity.</comment>
<file context>
@@ -173,6 +204,161 @@ export function runTypeScriptCase(testCase: DifferentialCase): DifferentialOutco
+ ...new Set(
+ nodes
+ .filter((node) => node.label !== null)
+ .map((node) => JSON.stringify([node.type, node.label])),
+ ),
+ ].sort();
</file context>
| (testCase) => | ||
| !testCase.swift && | ||
| ![ | ||
| 'interactive only compacts semantic representatives', |
There was a problem hiding this comment.
P3: The exclusion list in the RUNNER_UNSUPPORTED assertion (lines 75-76) and the inclusion list in the runner-presented filter (lines 89-90) are the same two case names written twice. Nothing ties them together: adding a future non-Swift case to only the exclusion list passes the deepEqual against Object.keys(RUNNER_UNSUPPORTED) while silently dropping that case from runner-presented coverage. Extract the pair into one shared constant (e.g. RUNNER_PRESENTED_NON_SWIFT_CASES) and reference it from both filters.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/ios-snapshot-differential.test.ts, line 75:
<comment>The exclusion list in the `RUNNER_UNSUPPORTED` assertion (lines 75-76) and the inclusion list in the runner-presented filter (lines 89-90) are the same two case names written twice. Nothing ties them together: adding a future non-Swift case to only the exclusion list passes the `deepEqual` against `Object.keys(RUNNER_UNSUPPORTED)` while silently dropping that case from runner-presented coverage. Extract the pair into one shared constant (e.g. `RUNNER_PRESENTED_NON_SWIFT_CASES`) and reference it from both filters.</comment>
<file context>
@@ -42,6 +61,103 @@ test('authored Swift and TypeScript golden cases agree', { timeout: SWIFT_RUN_TI
+ (testCase) =>
+ !testCase.swift &&
+ ![
+ 'interactive only compacts semantic representatives',
+ 'scroll indicator owned by a parent web view keeps list rows',
+ ].includes(testCase.name),
</file context>
| ].includes(testCase.name), | ||
| ) | ||
| .map((testCase) => testCase.name), | ||
| Object.keys(RUNNER_UNSUPPORTED), |
There was a problem hiding this comment.
P3: assert.deepEqual compares these arrays positionally, so the completeness guard silently depends on ordering: Object.keys(RUNNER_UNSUPPORTED) follows declaration order while fixture.cases follows the JSON file order. Inserting a new omitted case ahead of an existing one in the corpus, or reordering the declared keys, fails the guard even when the declared set is identical and complete, with a message that claims a missing asymmetry. Compare both sides as sorted sets so the contract is order-agnostic.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/ios-snapshot-differential.test.ts, line 80:
<comment>`assert.deepEqual` compares these arrays positionally, so the completeness guard silently depends on ordering: `Object.keys(RUNNER_UNSUPPORTED)` follows declaration order while `fixture.cases` follows the JSON file order. Inserting a new omitted case ahead of an existing one in the corpus, or reordering the declared keys, fails the guard even when the declared set is identical and complete, with a message that claims a missing asymmetry. Compare both sides as sorted sets so the contract is order-agnostic.</comment>
<file context>
@@ -42,6 +61,103 @@ test('authored Swift and TypeScript golden cases agree', { timeout: SWIFT_RUN_TI
+ ].includes(testCase.name),
+ )
+ .map((testCase) => testCase.name),
+ Object.keys(RUNNER_UNSUPPORTED),
+ 'every omitted authored case needs a declared Swift/runner asymmetry',
+ );
</file context>
| }; | ||
| const presented = presentIosSnapshot(input, request, { foldPolicy: testCase.foldPolicy }); | ||
| const published = canonicalNodes( | ||
| publishIosSnapshot(input, request, { foldPolicy: testCase.foldPolicy }).payload.nodes, |
There was a problem hiding this comment.
P3: runRunnerComposition presents the payload once via presentIosSnapshot and then calls publishIosSnapshot, which re-runs presentIosSnapshot internally on the same input and request (engine.ts line 31). Every runner-presented case therefore compacts and validates the full payload twice — per case, per fuzz seed. This doubles work that this test path controls directly; consider deriving the published payload from the single presentation, or documenting the double-present as intentional.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/capture-kit/src/ios-snapshot-engine/conformance-harness.ts, line 247:
<comment>`runRunnerComposition` presents the payload once via `presentIosSnapshot` and then calls `publishIosSnapshot`, which re-runs `presentIosSnapshot` internally on the same input and request (engine.ts line 31). Every runner-presented case therefore compacts and validates the full payload twice — per case, per fuzz seed. This doubles work that this test path controls directly; consider deriving the published payload from the single presentation, or documenting the double-present as intentional.</comment>
<file context>
@@ -173,6 +204,161 @@ export function runTypeScriptCase(testCase: DifferentialCase): DifferentialOutco
+ };
+ const presented = presentIosSnapshot(input, request, { foldPolicy: testCase.foldPolicy });
+ const published = canonicalNodes(
+ publishIosSnapshot(input, request, { foldPolicy: testCase.foldPolicy }).payload.nodes,
+ );
+ return { presented, published };
</file context>
| testCase.scope === null || | ||
| JSON.stringify(semanticMembership(canonicalNodes(actual ?? []))) === | ||
| JSON.stringify(semanticMembership(expected ?? [])) | ||
| ); | ||
| } |
There was a problem hiding this comment.
P3: qualityMembershipAgrees short-circuits whenever testCase.scope === null, so unscoped runner-presented cases never assert that the presented pipeline omits quality nodes. If Swift's presenter or the host stage: 'presented' adapter ever started emitting quality evidence for an unscoped corpus, this differential would not flag it (the qualityPayload would be validated for shape but never checked for emptiness). Compare against the empty expected membership instead of skipping the check.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/capture-kit/src/ios-snapshot-engine/conformance-harness.ts, line 300:
<comment>`qualityMembershipAgrees` short-circuits whenever `testCase.scope === null`, so unscoped runner-presented cases never assert that the presented pipeline omits quality nodes. If Swift's presenter or the host `stage: 'presented'` adapter ever started emitting quality evidence for an unscoped corpus, this differential would not flag it (the qualityPayload would be validated for shape but never checked for emptiness). Compare against the empty expected membership instead of skipping the check.</comment>
<file context>
@@ -173,6 +204,161 @@ export function runTypeScriptCase(testCase: DifferentialCase): DifferentialOutco
+ expected: readonly CanonicalNode[] | undefined,
+): boolean {
+ return (
+ testCase.scope === null ||
+ JSON.stringify(semanticMembership(canonicalNodes(actual ?? []))) ===
+ JSON.stringify(semanticMembership(expected ?? []))
</file context>
| testCase.scope === null || | |
| JSON.stringify(semanticMembership(canonicalNodes(actual ?? []))) === | |
| JSON.stringify(semanticMembership(expected ?? [])) | |
| ); | |
| } | |
| return ( | |
| JSON.stringify(semanticMembership(canonicalNodes(actual ?? []))) === | |
| JSON.stringify(semanticMembership(expected ?? [])) | |
| ); |
| acquired: DifferentialOutcome, | ||
| ): boolean { | ||
| if (swift.outcome !== acquired.outcome) return false; | ||
| if (swift.outcome === 'failure') |
There was a problem hiding this comment.
P3: The failure arm of runnerPresentationAgrees returns after comparing Swift's error to the acquired TypeScript error, before runRunnerComposition ever runs. For a runner-presented corpus that fails identically on both languages, the stage: 'presented' adapter — the pipeline this PR is meant to exercise — is never invoked, so an acquired/presented failure-code mismatch (e.g., the presented adapter throwing projection-mismatch while the acquired adapter reports the same typed error) passes silently. Run the presented composition in this arm too and assert its typed error agrees, instead of returning on the error comparison alone.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/capture-kit/src/ios-snapshot-engine/conformance-harness.ts, line 213:
<comment>The failure arm of `runnerPresentationAgrees` returns after comparing Swift's error to the acquired TypeScript error, before `runRunnerComposition` ever runs. For a runner-presented corpus that fails identically on both languages, the `stage: 'presented'` adapter — the pipeline this PR is meant to exercise — is never invoked, so an acquired/presented failure-code mismatch (e.g., the presented adapter throwing `projection-mismatch` while the acquired adapter reports the same typed error) passes silently. Run the presented composition in this arm too and assert its typed error agrees, instead of returning on the error comparison alone.</comment>
<file context>
@@ -173,6 +204,161 @@ export function runTypeScriptCase(testCase: DifferentialCase): DifferentialOutco
+ acquired: DifferentialOutcome,
+): boolean {
+ if (swift.outcome !== acquired.outcome) return false;
+ if (swift.outcome === 'failure')
+ return JSON.stringify(swift.error) === JSON.stringify(acquired.error);
+ const { presented, published } = runRunnerComposition(testCase, swift);
</file context>
|
Reviewed at da86813. The new runner-presented arm and fixtures look correct, and I found no blocking problem. Two questions you can take or leave: could CI: the Coverage failure is likely related. Changed-line coverage is 27.91% because |
thymikee
added
the
ready-for-human
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/capture-kit/src/ios-snapshot-engine/conformance.test.ts">
<violation number="1" location="packages/capture-kit/src/ios-snapshot-engine/conformance.test.ts:164">
P3: The slice-based assertions in this test hard-code the fixture's index layout: they assume indices 0-1 are App and Settings (with no 'General' label) and index 2 is the General Cell, but nothing validates those assumptions before `runnerPresentationAgrees` is called. A later fixture reorder or insertion (e.g., adding a node before the Cell, or a different voice-over ordering) silently changes which scenario the slices exercise, failing loudly for the wrong reason or passing when the intended membership scenario no longer holds. Assert the sliced sources' expected labels/types (or slice by label instead of position) ahead of the comparison calls.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
|
|
||
| assert.equal(runnerPresentationAgrees(testCase, swift(source.nodes), acquired), true); | ||
| assert.equal( | ||
| runnerPresentationAgrees(testCase, swift(source.nodes.slice(0, 3)), acquired), |
There was a problem hiding this comment.
P3: The slice-based assertions in this test hard-code the fixture's index layout: they assume indices 0-1 are App and Settings (with no 'General' label) and index 2 is the General Cell, but nothing validates those assumptions before runnerPresentationAgrees is called. A later fixture reorder or insertion (e.g., adding a node before the Cell, or a different voice-over ordering) silently changes which scenario the slices exercise, failing loudly for the wrong reason or passing when the intended membership scenario no longer holds. Assert the sliced sources' expected labels/types (or slice by label instead of position) ahead of the comparison calls.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/capture-kit/src/ios-snapshot-engine/conformance.test.ts, line 164:
<comment>The slice-based assertions in this test hard-code the fixture's index layout: they assume indices 0-1 are App and Settings (with no 'General' label) and index 2 is the General Cell, but nothing validates those assumptions before `runnerPresentationAgrees` is called. A later fixture reorder or insertion (e.g., adding a node before the Cell, or a different voice-over ordering) silently changes which scenario the slices exercise, failing loudly for the wrong reason or passing when the intended membership scenario no longer holds. Assert the sliced sources' expected labels/types (or slice by label instead of position) ahead of the comparison calls.</comment>
<file context>
@@ -132,6 +144,87 @@ test('the differential TypeScript runner preserves typed failures', () => {
+
+ assert.equal(runnerPresentationAgrees(testCase, swift(source.nodes), acquired), true);
+ assert.equal(
+ runnerPresentationAgrees(testCase, swift(source.nodes.slice(0, 3)), acquired),
+ true,
+ 'Swift may delegate Button and StaticText to the Cell representative',
</file context>
|
Reviewed at b934eb2. The delta since da86813 only exports Not blocking: the CI: Smoke Tests is still running and the other 18 checks pass. This diff does not touch the route that Smoke exercises. |
|
1 participant
Summary
Exercise the Swift presenter through the TypeScript
stage: 'presented'adapter, semantic compaction, and publication. Keep the acquired route and shared authored corpus; add reduced Settings, TextView, WebView, Cell, and Safari captures. The runner arm checks semantic membership, source representatives, scoped quality, and clipping without requiring array or index equality. Closes #2974.Eight files changed. Baseline
c58fe851e2c289c07b97b853fe89cd60af122d59; headb934eb23dc63293fa9451be510956f5cb6963196. Rename-aware diff: 699 added, 294 deleted, gross 993; production code and behavior unchanged. The 267-line runner fixture move is test-only.Validation
At
b934eb23d: focused Vitest 8 passed; full coverage suite 1,559 files and 12,533 tests passed; changed-line coverage 42/43 (97.67%). Swift package 17 passed; differential 12 passed in 2.49 s, including deterministic seeds and 10 acquired/12 runner-presented authored cases. Five captured interactive cases passed. An ancestor-ownership mutation changed these from 5 passed to 2 passed/3 failed (TextView, WebView, Cell); Safari same-frame stayed green. Mutation restored.pnpm formatand exact-headpnpm check:affected --runpassed.All head CI checks now pass, including iOS smoke on rerun. Its first attempt exhausted a 10-second wait during
runner-startbefore capture; a later failure snapshot showed the awaited label in a healthy XCTest tree. The cross-branch smoke harness budget is addressed separately in #3026.