refactor(daemon): route perf metrics sampling body through PlatformPlugin facet

[devin-ai-integration]

Summary

Closes the last daemon-owned platform branch in perf handling (issue #1188). buildPerfResponseData still dispatched the metrics-sampling body by hand:

if (session.device.platform === 'android') { await applyAndroidPerfMetrics(...); return; }
await applyApplePerfMetrics(...);

The perf facet already carried the support gate (supportsMetrics); this extends it with a neutral sampler discriminant and routes the sampling body through it, mirroring the RecordingBackendTag pattern byte-for-byte:

• New daemon-owned neutral tag PerfMetricsSamplerTag = 'apple' | 'android' and an exhaustive PERF_METRICS_SAMPLERS_BY_TAG map (daemon keeps ownership of sampler composition).
• PlatformPlugin.perf gains metricsSamplerTag(device) (type-only import of the tag, exactly like recording). Apple plugin returns 'apple', Android returns 'android'; web/linux carry no perf facet (unchanged).
• buildPerfResponseData now does resolvePerfMetricsSampler(device) → tag lookup → sampler, after the support + app-bundle gates. The two apply*PerfMetrics wrappers are gone; applySampledPerfMetrics is shared.

Public behavior is byte-identical — no new metrics, no daemon-owned types leaked into platforms. The tag is consulted only after supportsPlatformPerfMetrics admits the platform, so the facet is always present; the undefined fallthrough preserves the former base response.

Parity is pinned in perf-plugin-routing-parity.test.ts against an independent verbatim copy of the former branch (device.platform === 'android' ? 'android' : 'apple') across the exhaustive device matrix, plus assertions that the factless families expose no metricsSamplerTag.

Removes the now-resolved b.3 perf sampling body deferred note from CONTEXT.md.

Verification

format:check, typecheck, lint, layering, and Fallow (--base origin/main) all clean. test:unit (3532 tests), the perf parity test, apple/android perf suites, request-router-android-perf, provider-integration (apple-leak guard), test:smoke, and pnpm build all pass on the CI Node 24.13 toolchain.

Link to Devin session: https://app.devin.ai/sessions/4f6c547c71ac41a899c0ece81403bd6f
Requested by: @thymikee

[devin-ai-integration]

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

• Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
• Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

• Disable automatic comment, CI, and merge conflict monitoring

[github-actions]

Size Report

Metric	Base	Current	Diff
JS raw	1.6 MB	1.6 MB	+64 B
JS gzip	517.9 kB	517.9 kB	+51 B
npm tarball	624.2 kB	624.2 kB	+46 B
npm unpacked	2.2 MB	2.2 MB	+64 B

Startup median (7 runs, lower is better):

Scenario	Base	Current	Diff
CLI --version	27.6 ms	28.2 ms	+0.6 ms
CLI --help	56.4 ms	57.5 ms	+1.1 ms

Top changed chunks:

Chunk	Raw diff	Gzip diff
dist/src/session.js	+2 B	+31 B
dist/src/internal/daemon.js	+62 B	+20 B

[thymikee]

Review — 🟢 SHIP

A clean, minimal refactor. Moves per-platform perf metrics sampler selection out of the hand-coded device.platform === 'android' branch in buildPerfResponseData and routes it through the PlatformPlugin.perf facet via a neutral PerfMetricsSamplerTag ('apple' | 'android'), with the daemon retaining sampler ownership through PERF_METRICS_SAMPLERS_BY_TAG. Mirrors the existing RecordingBackendTag facet pattern byte-for-byte; the sampling body itself is unchanged. Verified on the PR branch:

• Behavior preserved / await intact. The new hot path const results = await sampler(session, session.appBundleId, options); applySampledPerfMetrics(...) is functionally identical to both old arms. Error propagation (Promise.allSettled, settleMetric) untouched.
• No runtime-undefined path. metricsSamplerTag is a required method on the perf facet, so any plugin exposing perf must implement it (Apple + Android do). The resolver is only reached after supportsPlatformPerfMetrics admits the platform via the same facet, so the tag is always present when consulted; the undefined fallthrough only guards the unreachable case.
• Type-only core→daemon edge. core/platform-plugin/plugin.ts imports PerfMetricsSamplerTag from daemon/handlers/session-perf.ts as import type (erased), exactly like the pre-existing RecordingBackendTag import. No runtime cycle.
• Exhaustiveness. PERF_METRICS_SAMPLERS_BY_TAG is typed Record<PerfMetricsSamplerTag, …>, so adding a tag without a sampler is a compile error. Parity test pins the tag to a verbatim copy of the former branch across the full device matrix and asserts linux/web expose no metricsSamplerTag.
• Per-sample overhead is one registry lookup + one property access — negligible.

The Apple sampler is 2-arity and silently ignores the newly-passed options 3rd arg (TS param-count contravariance) — benign, matches old behavior. No spawnSync/exec.ts violations. Advances the PlatformPlugin consolidation by retiring the last daemon-owned perf branch (ADR-0008/0009). No findings.

_{🤖 Generated with Claude Code}

[thymikee]

Review finding (P2): the new sampler routing is not exercised by the claimed end-to-end test. perf-plugin-routing-parity.test.ts calls buildPerfResponseData with no appBundleId, so execution returns through applyMissingAppPerfMetrics before resolvePerfMetricsSampler is reached (session-perf.ts:150-157). The separate tag-table assertions prove the facet data matches the former branch, but the suite still passes if production keeps/restores the old device.platform === "android" sampler branch. Please add a routed test that reaches sampler selection and fails when the new facet lookup is bypassed; issue #1188 needs shipped-path evidence, not only registry parity.

[devin-ai-integration]

Re: the P2 finding on the routing test — good catch. The parity suite only proved registry parity and never reached the shipped dispatch (buildPerfResponseData returned through applyMissingAppPerfMetrics before resolvePerfMetricsSampler, since the test passed no appBundleId). Fixed in 698d0c1 with two routed tests:

• buildPerfResponseData dispatches the Android sampler selected by the facet — sets appBundleId, injects a scripted androidAdb, and asserts it is invoked (the Android sampler is the only arm that threads options.androidAdb) and that all three metrics carry the sampler's failure reason, i.e. not the base/missing-app response. This runs through the real resolvePerfMetricsSampler → PERF_METRICS_SAMPLERS_BY_TAG path with no device I/O.
• buildPerfResponseData consults the sampler only past the missing-app guard — without an appBundleId the scripted adb is never touched, pinning the guard order.

Verified it fails when the facet lookup is bypassed: flipping the Android plugin to metricsSamplerTag: () => 'apple' routes Android to the Apple sampler, the adb spy never fires, and both the routed test and the tag-parity test fail. So the suite now fails on a broken facet tag and a broken resolver — shipped-path evidence, not only registry parity. Apple's 'apple' → Apple sampler mapping stays compile-guaranteed by the exhaustive Record<PerfMetricsSamplerTag, …>.

[thymikee]

Re-review finding (P2): the new test reaches sampling, but still does not prove facet routing. The injected-ADB assertions exercise the Android sampler, yet the removed device.platform === "android" production branch invokes that same sampler with the same options and produces identical calls/results. Restoring the old branch leaves the new test green. Please make the test observable depend specifically on metricsSamplerTag/facet selection so reverting the production lookup fails; otherwise issue #1188 remains unguarded despite the extra coverage.

[devin-ai-integration]

Re the re-review P2 (facet routing not yet proven) — fixed in 33ecb9c.

Added a test that varies the facet independently of the device platform: it overrides the Apple plugin's metricsSamplerTag to 'android' (restored in finally) and asserts buildPerfResponseData on an Apple simulator routes to the Android sampler (the scripted adb fires). Only a facet-driven selection can produce that — the removed device.platform === 'android' branch keyed on the platform, so it would keep an Apple device on the Apple sampler and never touch the adb.

Mutation check — reverting resolvePerfMetricsSampler to device.platform === 'android' ? …android : …apple leaves the other 8 tests green but fails this one:

× buildPerfResponseData selects the sampler by the facet tag, not the device platform
  assert.ok(calls() > 0, 'facet tag routed the Apple device to the Android sampler')
  - true
  + false

So restoring the production lookup now fails the suite, closing the gap on issue #1188.

[thymikee]

Final re-review at 33ecb9c: no actionable findings remain. The new mutation-style test now fails if the deleted device.platform sampler branch is restored, proving the shipped route depends on the plugin facet; production behavior stays otherwise unchanged and all checks are green. Marking ready-for-human.

[github-actions]

PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-07-10 13:27 UTC