Skip to content

Commit dedd8e8

Browse files
thymikeethymikee
committed
refactor(snapshot): answer the review round on sharing, parity, and evidence
The eager-closure refusal the review asked to be pointed at or filed is not a table of budgets: the gate ratchets every entry's closure against the committed merge-base tree, so the numbers to read are 9 for `facades/capture.ts` and 2 for `snapshot-quality-verdict.ts`. Re-installing one shared kernel predicate for both readers on this head fails six entries and names both of those, which is why the vocabulary stays a map per reader; recorded on #2872 as the accepted deviation. The `android-helper` asymmetry is not real: `SNAPSHOT_QUALITY_BACKEND_CAPABILITIES` merges the Android declaration, so both readers accept that strategy and the parity table now carries the row instead of omitting it. The client-side drop of an unknown state or backend gets its CHANGELOG entry, the justification comments shrink to one constraint sentence each, the kernel test keeps only its `@ts-expect-error`, and the word left dangling by the last rename goes.
1 parent 3fc635c commit dedd8e8

6 files changed

Lines changed: 24 additions & 38 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,13 @@
22

33
## Unreleased
44

5+
- Changed (snapshot): a snapshot quality verdict whose `state` or `backend` this version does not
6+
declare now reads as verdict-absent on the serialized response path, where it used to be cast into
7+
the verdict type. A caller can no longer be handed a degradation presented under a state or a
8+
recovery strategy nobody declared; that case falls back to the tree-shape detectors exactly as a
9+
missing verdict does. The Apple runner stamps `state` from a closed enum
10+
(`healthy | recovered | sparse`) and the wire format is unchanged, so only a version-skewed runner
11+
or a hand-written payload reads as absent now. (#2872)
512
- Fixed (ios): `perf cpu profile report --kind xctrace` on Xcode 27 no longer fails with
613
`Apple xctrace CPU report contained no samples` on a trace that holds thousands of samples. Xcode
714
27 exports each `time-profile` sample stack as `<tagged-backtrace>` instead of `<backtrace>`, and

‎packages/capture-kit/src/snapshot-quality-verdict.test.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,7 @@ const VERDICT_PAYLOADS: unknown[] = [
134134
{ state: 'sparse', backend: 'tree', timing: { acquisitionMs: 12.5 } },
135135
{ state: 'sparse', backend: 'tree', timing: { acquisitionMs: 12.5, presentationMs: 34.75 } },
136136
{ state: 'sparse', backend: 'tree', reasonCode: 'future-code' },
137+
{ state: 'recovered', backend: 'android-helper', reasonCode: 'requested-backend' },
137138
{ state: 'degraded', backend: 'tree' },
138139
{ state: 'sparse', backend: 'uiautomator' },
139140
{ state: 'constructor', backend: 'constructor' },

‎packages/capture-kit/src/snapshot-quality-verdict.ts‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,10 @@ import type { SnapshotQualityState, SnapshotQualityVerdict } from '@agent-device
22
import { SNAPSHOT_QUALITY_BACKEND_CAPABILITIES } from './snapshot-quality-backend-capabilities.ts';
33

44
/**
5-
* The verdict names this version can speak, each keyed against its kernel union so a map cannot
6-
* fall behind it: a name added there without a key here is a compile error, where a set literal
7-
* merely typed as the union stays green and this reader drops the verdict as verdict-absent. These
8-
* readers hold the maps rather than importing the kernel's, because this module's eager closure is
9-
* frozen at its merge-base size. The strategies need no map: `SNAPSHOT_QUALITY_BACKEND_CAPABILITIES`
10-
* is the accepted set, keyed by the same names.
5+
* The verdict names this version can speak, keyed against the kernel unions so a map cannot fall
6+
* behind one. They cannot be one shared kernel predicate: this module's eager closure is frozen at
7+
* its merge-base size (#2872). The strategies need no map — `SNAPSHOT_QUALITY_BACKEND_CAPABILITIES`
8+
* is already keyed by exactly those names.
119
*/
1210
const DECLARED_STATES: Record<SnapshotQualityState, true> = {
1311
healthy: true,

‎packages/contracts/src/snapshot-capture-annotations.ts‎

Lines changed: 7 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,9 @@ import type {
77
import type { AndroidSnapshotBackendMetadata } from './snapshot-types.ts';
88

99
/**
10-
* The two verdict names this host has to be able to speak: `state` decides whether a capture reads
11-
* as degraded, and `backend` names the recovery strategy in the warning line. Each map is keyed
12-
* against its kernel union, so a name added there without a key here is a compile error, where a set
13-
* literal merely typed as the union stays green and a runner's verdict is dropped as
14-
* verdict-absent. The maps live here rather than behind a kernel import because
15-
* `facades/capture.ts` pins its eager module closure and `kernel/snapshot.ts` is not in it.
10+
* The verdict names this host has to speak, each keyed against its kernel union so a name added
11+
* there without a key here is a compile error. They cannot be one shared kernel predicate: the
12+
* eager-closure gate keeps `kernel/snapshot.ts` out of `facades/capture.ts` (#2872).
1613
*/
1714
const DECLARED_STATES: Record<SnapshotQualityState, true> = {
1815
healthy: true,
@@ -114,15 +111,10 @@ function readTargetActivation(value: unknown): IosTargetActivation | undefined {
114111
}
115112

116113
/**
117-
* Re-read of a fact this module published, in the shape `readTargetActivation` above also uses: the
118-
* two names that decide presentation are checked, and the verdict is forwarded as published. Named
119-
* apart from capture-kit's stricter `readSnapshotQualityVerdict`, which normalizes an untrusted
120-
* runner payload and reads every field. Reading
121-
* This one cannot share that code — the eager-closure gate freezes both readers' module closures and
122-
* the duplication gate refuses a second normalization — so the pair is pinned together by
123-
* `snapshot-quality-verdict.test.ts`. What stays guaranteed here is the part only this boundary can
124-
* check: a name this version cannot speak reads as verdict-absent, so a version-skewed runner cannot
125-
* hand the host a degradation it would present under a state or strategy nobody declared.
114+
* Re-read of a fact this module published, in the shape `readTargetActivation` above uses: the two
115+
* names that decide presentation are checked, the rest is forwarded as published. capture-kit's
116+
* `readSnapshotQualityVerdict` normalizes an untrusted runner payload field by field; the two
117+
* readings are pinned to each other in `snapshot-quality-verdict.test.ts`.
126118
*/
127119
function readPublishedSnapshotQualityVerdict(value: unknown): SnapshotQualityVerdict | undefined {
128120
if (!value || typeof value !== 'object') return undefined;

‎packages/kernel/src/snapshot-quality-states.test.ts‎

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -31,19 +31,7 @@ test('the declared verdict states are the shared wire vocabulary', () => {
3131
);
3232
});
3333

34-
/**
35-
* The union the readers key their exhaustive maps against: it admits exactly the declared states,
36-
* so an undeclared one cannot reach a verdict and a state added to the tuple reaches every map.
37-
*/
3834
test('the verdict state type admits exactly the declared states', () => {
39-
const declared: Record<SnapshotQualityState, true> = {
40-
healthy: true,
41-
recovered: true,
42-
sparse: true,
43-
};
44-
for (const state of SNAPSHOT_QUALITY_STATES) {
45-
assert.equal(declared[state], true, state);
46-
}
4735
// @ts-expect-error a state nobody declared cannot enter the verdict type
4836
const undeclared: SnapshotQualityState = 'degraded';
4937
void undeclared;

‎packages/kernel/src/snapshot.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,11 @@ export type SnapshotQualityTiming = {
2727
/**
2828
* The verdict states a capture plan may stamp. This tuple is the ONE declaration of that
2929
* vocabulary, and `SnapshotQualityVerdict['state']` is its projection; readers hold exhaustive maps
30-
* over the union rather than importing this module, because the eager-closure gate freezes their
31-
* loading shape. This tuple and the Apple runner's `SnapshotQualityState.allCases` are each pinned
32-
* to `contracts/fixtures/ios-snapshot-quality-states.json` as a set, so a state one side renames,
33-
* adds, or deletes without the other goes red there instead of arriving as a verdict the host
34-
* cannot name — which reads as verdict-absent and drops the disclosure with it.
30+
* over the union instead of importing this module, because the eager-closure gate freezes their
31+
* loading shape (#2872). This tuple and the Apple runner's `SnapshotQualityState.allCases` are each
32+
* pinned as a set to `contracts/fixtures/ios-snapshot-quality-states.json`, so a state one side
33+
* renames, adds, or deletes without the other goes red there instead of arriving as a verdict the
34+
* host cannot name — which reads as verdict-absent and drops the disclosure with it.
3535
*/
3636
export const SNAPSHOT_QUALITY_STATES = ['healthy', 'recovered', 'sparse'] as const;
3737

0 commit comments

Comments
 (0)