ADFA-5534: attach the carousel metrics file to user feedback

[davidschachterADFA]

A report of "it got slow" arrives with nothing to correlate it against. The feedback send now carries the session's own metrics, gzipped, alongside the screenshot and log it already attaches.

Three commits: move the assembly off the carousel, add the gzipped writer and an emptiness check, wire the feedback path.

The part that was actual work

The snapshot assembly began on MetricsCarouselController, which reads the binding for a Context and returns early when the carousel is unbound. That is right for a button on the carousel and wrong for everything else that wants the file — the feedback FAB can be tapped with the strip closed, and a crash report (ADFA-5526) is assembled with no UI at all. So it moved to MetricsSnapshotAssembler, which needs only the three watchers, the annotation store and a Context. That is what makes the format module reusable rather than carousel-bound, and it is the piece both remaining consumers need.

Module boundary

FeedbackManager and FeedbackEmailHandler are in common; the metrics are in app, which depends on common and not the reverse. So the feedback code cannot reach the metrics — it takes a provider, mirroring the getLogContent hook already there for the log.

Suspending, unlike that one: the snapshot must be read on the main thread and the compressed file written off it, and neither belongs in a click listener. sendFeedbackWithAttachments is already inside lifecycleScope.launch, so it drops straight in. The call is guarded — feedback about a broken IDE has to send even if this part fails.

FeedbackEmailHandler already built ACTION_SEND_MULTIPLE from a URI list and fell back to ACTION_SENDTO on an empty one, so a third attachment is one more URI. FeedbackButtonManager's constructor gains @JvmOverloads, because TermuxActivity constructs it from Java, where Kotlin's default arguments are invisible.

The two tickets contradicted each other

ADFA-5531's format section says a file is always produced; this ticket says omit the attachment when there are no data rows. Both are satisfiable — the writer always writes, the caller asks first — so MetricsCsv.Snapshot.hasRows is how a caller asks, and I have added rule (d) to ADFA-5531 spelling out that sending an empty file is the caller's decision and not the format's. ADFA-5526 will inherit that rather than the confusion.

Compression

There was no compression of any kind in the repo before this — no GZIPOutputStream, ZipOutputStream or Deflater in app or common. ADFA-5526 says "compress, as this ticket does", but 5526 is blocked and this one is not, so this is where it gets decided:

• gzip, .csv.gz, streamed through GZIPOutputStream so the uncompressed megabyte never has to exist.
• Not zip. It is one file, so an archive container adds a name and nothing else, and java.util.zip gives us either with no dependency. The only thing zip buys is double-click friendliness for a file we run scripts over.
• Its own directory. The exports prune to three, and a feedback send must not evict an export the user is part-way through handing to another app.

Measured on a real 86-row attachment: 6425 → 1395 bytes, 4.6x. The rows are near-identical by nature — the timestamp advances by a constant and the magnitudes barely move — so a full buffer does considerably better. A test pins the ratio at better than 4x, so if that ever stops being true the extra step gets questioned rather than kept out of habit.

Verification

On a Pixel 6 Pro, a real feedback send:

• writes cache/metrics-reports/2026_09_07_00_52_54_706.csv.gz;
• opens the chooser with act=android.intent.action.SEND_MULTIPLE typ=message/rfc822 clip={message/rfc822 3 items};
• the URI grants name all three — the metrics .csv.gz, Feedback Log ….txt, Screenshot ….jpg;
• the attachment gunzips to a valid 86-row CSV with the expected header.

The empty case is unit-tested rather than device-tested, deliberately: clearing the buffers via a sampling-rate change is the only way to reach it, and the next sample lands about a second later because the in-flight delay() still holds the old interval — so the window is shorter than a tap. I tried it and caught exactly one row, which is the writer behaving correctly rather than the check failing.

Unit tests: hasRows both ways, gzip round-trips to the identical CSV, the compression ratio, the two directories pruning independently, and both staying bounded. Full :app unit suite and spotlessCheck green.

One thing I fixed while writing those: the filename assertions initially passed only because this machine is in America/Los_Angeles. They now pass an explicit zone, so they do not depend on where CI runs.

Font scale

No UI added — the feedback FAB and its dialog are unchanged. Nothing to re-verify at 2.0.

Review fixes

This PR was red on its own test. both directories stay bounded fails here: pruneTo picked the oldest n across every file and then skipped the one just written, so it deleted one too few whenever that one sorted into the set, and the directory crept one over the limit each time. The fix was sitting two branches up on ADFA-5526. It belongs where the test that catches it is, so it has moved here.

The tie-case test that came with it was vacuous, and is rewritten. Tying every existing file to one past timestamp does not reproduce the bug: the file written last still carries a real mtime, so it sorts last, is never in the deleted set, and the skip never fires. Dating the existing files into the future puts the new one at the front of the sort deterministically. It now fails without the fix — four files where three are allowed — as does the bounded test.

The compression-ratio test measured a straight line. It ran on a snapshot whose every column advanced by a constant, which gzip compresses about fiftyfold, so "better than 4x" was met with room to spare by data no session produces. It now runs on a seeded session whose columns move the way a device's do — memory in steps of megabytes, network in bursts, temperature and power drifting, thermal status flipping. That is deliberately noisier than reality (2.8x here against the 4.6x measured on a real attachment), so it is a floor. The bound is halving, which compression bypassed fails and a shift in gzip's tuning does not.

Also carries ADFA-5531's row-alignment fix into MetricsSnapshotAssembler, which is where the assembly moved on this branch.

Review fix: the whole attachment step is inside the guard it claims

The comment said feedback about a broken IDE must still send if the metrics part fails. Two ways it did not:

• fileProviderUriFor was chained outside the runCatching. FileProvider throws IllegalArgumentException for a path outside its configured roots, and this feature added a directory — so the throw went past the guard and took the whole send with it. Unreachable today only because the paths XML has a root-path of ".", which is not a thing to depend on.
• CancellationException was swallowed. runCatching catches Throwable and the provider genuinely suspends — withContext twice — so an activity destroyed mid-write had its cancellation eaten and the coroutine ran on to startActivity() on a dead activity.

The step is extracted to metricsAttachmentUri so it can be tested at all: sendFeedbackWithAttachments needs a live activity and its lifecycle scope, and this is the part with the failure modes. Five tests; the two that name these defects fail against the old shape.

On device

Verified on a Galaxy Note20 Ultra (SM-N986U, Android 13, arm64), running a local integration build of this stack plus ADFA-5514.

A real feedback send from the FAB produced:

act=android.intent.action.SEND_MULTIPLE  typ=message/rfc822
clip={message/rfc822 3 items: {U(content)…

Three attachments, and the gzipped metrics file landed in its own directory — cache/metrics-reports/2026_09_07_14_52_32_316.csv.gz, 13,187 bytes — not in metrics-exports alongside the user's own exports. That separation is the point of the second directory, and it holds in practice: a plain export taken three minutes earlier is still sitting untouched in metrics-exports.

Compression on real data: 13,187 bytes against a 54,528-byte plain export over a comparable window, roughly 4x.

One thing worth a look before merge. The send logs a Bundle warning from FeedbackManager.kt:383, which is the startActivity call in sendFeedbackWithAttachments. The intent went through and this is a warning rather than a failure — but that line is exactly the path guarded against TransactionTooLargeException, and this change adds a third attachment to the payload. I have not chased it; flagging it rather than leaving it in a log nobody reads.

🤖 Generated with Claude Code

https://claude.ai/code/session_017CCQUU7tBzZL61EmQhJP8j

[claude]

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.

_{Tip: disable this comment in your organization's Code Review settings.}