Skip to content

Diagnostics: Stop reporting an empty capture as a withheld one - #31

Merged
d4rken merged 1 commit into
mainfrom
contribution-empty-report
Jul 27, 2026
Merged

Diagnostics: Stop reporting an empty capture as a withheld one#31
d4rken merged 1 commit into
mainfrom
contribution-empty-report

Conversation

@d4rken

@d4rken d4rken commented Jul 27, 2026

Copy link
Copy Markdown
Member

What changed

When the contribution wizard captures two OEM modes and finds no differences between them, the report it generates now says exactly that, instead of "(no settings approved for inclusion)" — which read as though the contributor had chosen to hide every row.

The report also states what was measured: how many modes were captured, which settings providers were compared, and how many settings actually differed.

Two ways to produce an empty report are now handled:

  • A single captured mode can no longer advance to the review step. With nothing to compare against, it could only ever produce a report with no data in it.
  • A two-mode capture that finds nothing now shows a card explaining the likely causes — the mode wasn't actually switched between captures, it was switched but captured before the system applied it, or the ROM keeps the setting somewhere Amply can't read — with a "Start over" button. It stays sendable, because the third case is a genuine finding, but the button now reads "Continue anyway".

A capture that manages to read nothing at all is now reported as a failed capture rather than passing silently as a valid result.

Technical Context

Closes the reporting half of #23, where a HyperOS 3 report arrived with an empty matrix and misleading wording.

Why the wording mattered. formatContributionReport emitted one line for two unrelated outcomes: an empty matrix (a measurement result) and an all-withheld matrix (a contributor's privacy choice). The only discriminator was the absence of the optional withheld_rows line — undocumented, and easy to misread as a redaction that never happened. changed_rows now makes it explicit.

Schema 1 → 2. scanned_namespaces, captured_mode_count, and changed_rows are new. Nothing parses these reports (they're read by humans in issues), so there's no migration concern. The new fields are counts and fixed namespace names only — no key or value can reach them, so the privacy path is unchanged: the pure formatter still never sees a raw snapshot.

The single-capture path was the worse bug. deriveMatrix returns rows only where a value set has more than one distinct entry, so one observation yields an empty matrix by construction. goNext() gated on observations.isNotEmpty(), so a one-mode session walked straight through review into a public issue with no data. The ViewModel guard is authoritative (observations.size >= MIN_MODES); the bottom bar mirrors it so the button is visibly disabled rather than inert.

Non-blocking was deliberate. An empty result on an unknown ROM is exactly the signal that its charging mode isn't in the AOSP providers. Blocking delivery would suppress the one case worth hearing about, so the friction is a warning card plus a relabeled action, not a modal.

Capture health. ShizukuSettingsBackend.snapshot() only returns Failure when the binder call throws; a command returning empty or unparsable output parses to an empty map and becomes Success. Three of those merge into an empty snapshot that diffs to "nothing changed" — indistinguishable from the real finding above. The guard triggers only when all three namespaces merge to zero keys, so a legitimately sparse single namespace is unaffected.

Review guidance. ContributionReport.kt carries the discrimination logic; the rest is guards and presentation. The plan and the implementation were both reviewed by Codex; the single-capture gap and the capture-health hole came out of that review.

Testing

  • 751 unit tests × both flavors, 0 failures; lintVital{Foss,Gplay}Release and assemble{Foss,Gplay}Debug green.
  • New coverage: both empty-case wordings, the new report fields, schema pin, one-capture-cannot-advance, identical-captures-yield-empty-review, restart-from-empty, empty-snapshot-is-a-failure, plus Compose tests for the warning card, the relabeled action, and the disabled/enabled Next states.
  • No device run — this touches report text and wizard navigation only, no charge-control path.

A contribution report with no rows printed "(no settings approved for
inclusion)" regardless of cause. In issue #23 the matrix was empty --
nothing differed across the captured modes -- but the wording reads as a
contributor redacting every row, so the only way to tell the two apart
was the absence of an optional withheld_rows line.

Report schema 2 states what was measured: scanned_namespaces,
captured_mode_count, and changed_rows. An empty matrix now says so in
its own words and names the providers that were actually compared;
all-withheld keeps the inclusion wording.

The wizard also let two paths reach that report unchallenged. A single
capture could advance to review, where a diff has nothing to diff
against and the matrix is empty by construction -- now blocked at two
observations in the ViewModel, with the UI mirroring the guard. A
genuinely empty two-mode result stays deliverable, because "this ROM
keeps the mode elsewhere" is a real finding, but the review step
explains the likely causes, offers a restart, and relabels the primary
action to "Continue anyway".

A backend only reports Failure when the call threw; empty or unparsable
output parses to an empty map instead, and three empty namespaces would
diff to "nothing changed" and look like that same real finding. A merged
snapshot with no keys at all is now a capture failure.
@github-actions github-actions Bot added the enhancement New feature or request label Jul 27, 2026
@d4rken d4rken added bug Something isn't working and removed enhancement New feature or request labels Jul 27, 2026
@d4rken
d4rken merged commit db3839a into main Jul 27, 2026
12 checks passed
@d4rken
d4rken deleted the contribution-empty-report branch July 27, 2026 10:11
d4rken added a commit that referenced this pull request Jul 28, 2026
The "Help add support" wizard no longer produces reports a maintainer
cannot act on.

Feature name and software version are now required before capturing.
They are the only part of a report that survives a zero-diff capture and
often identify the ROM family on their own, so they move ahead of CAPTURE
rather than sitting optional on a step users skip.

Delivery now requires the report to carry at least one setting, which
closes two dead ends with one predicate. An empty matrix was deliverable
on purpose in #31, because "this ROM keeps the mode elsewhere" is a real
result, but in practice it is indistinguishable from the two capture
mishaps that produce identical output. The second dead end is a matrix
whose rows were all withheld: a new device's key is never on the
auto-disclose allowlist by construction, so the reports worth the most
are exactly the ones that arrive empty when the contributor skips the
reveal step. Both cases now get a card naming the cause instead of a
"Continue anyway" button or an unexplained disabled Next. Withholding
individual rows is untouched.

Also adds docs/support/kindmail-system-prompt.md, the support assistant's
Amply knowledge: device-support matrix, discovery-report triage, and the
pre-launch overrides for the shared prompt's Pro/refund/Play sections.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant