Correct six documentation claims the overnight review falsified #45

Merged
JMR-dev merged 4 commits from docs/review-corrections into main 2026-08-23 04:21:33 +00:00
4 Commits
Author SHA1 Message Date
JMR-devandClaude Opus 5 614af35647 Hold the corrections themselves to the standard they impose
Three defects in the three preceding commits, found on review. A commit set
whose subject is stale dates and inferred status cannot carry either.

Dates. Both correction blocks were stamped 2026-08-23. The commits are dated
2026-08-22, as is every other date in these two files and the review that
produced them -- a day in the future, in the one place a reader checks to see
how fresh a correction is. Corrected to the commit date, and the D6 note now
carries one too.

Coherence. The Status line was changed to say fix status "tracks main,
re-checked at 18c53a3" while "Last verified: 2026-08-22, against main at
903b43c" stood two lines below it, unchanged. A reader would take the whole
document as anchored to 903b43c -- exactly the failure being corrected. The two
anchors now say what each covers and that they move independently: the as-found
bodies are frozen at 903b43c, the status is not.

Inferred count. "detekt 0, lint clean" was carried over from the old text on the
strength of a BUILD SUCCESSFUL, which means "nothing above threshold", not
"nothing found" -- and this project deliberately keeps a real lint finding
visible (`informational += "UsableSpace"`), so "clean" was wrong as well as
unmeasured. Read off the reports instead: detekt.xml has zero <error> elements,
lint-results-debug.txt says 0 errors, 0 warnings, 1 hint. Stated that way, which
is also the convention the audit's own opening table already uses.

README's correction note is tightened from nine lines to six. It sits on the
first screen and nothing load-bearing is dropped.

Gate green: ktlintCheck, detekt, lintDebug.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-22 23:01:43 -05:00
JMR-devandClaude Opus 5 01cbc94888 Say what the FFmpeg format tests have actually been run against
R18 / #27. The front-page status line said the FFmpeg format tests "have been
written but not yet executed on a device". They have been executed, repeatedly
and green, and this is the one line a contributor uses to decide whether the
FFmpeg path is trustworthy -- understating it costs more than a stale detail
elsewhere would.

FFmpegEngineTest's nine format tests -- mp3, gif, matroska, flac, wav, opus,
H.264, H.265, and the one asserting a failure surfaces as an exception rather
than a silent empty file -- were present at every commit cited below, checked by
counting @Test in that file at each:

- Physical Pixel 10 Pro XL, API 37, 2026-08-21 at edd6385: 40 / 0 / 0 / 2.
- The audit's own Pixel pass, 2026-08-22: 49 / 0 / 0 / 2.
- Four local emulator levels, API 33-36, 2026-08-22 at 22c7914: 49 / 0 / 0 / 2
  each.

Zero failures in all of them, and the two skips are RealMediaBenchmark's
assumption-guarded tests, not these. CI's e2e matrix in status_check.yml covers
API 33-36 and the workflow triggers on pull_request against main, so "on every
pull request" is checked rather than assumed.

The replacement names the runs instead of a count, since a total is the part
that goes stale -- androidTest has moved 40 to 49 to 57 in a few days. It also
keeps the API 37 caveat visible: no CI row, so that level stays a manual Pixel
check before each release.

Held, not decided here: whether a front-page status line should carry a
verification claim at all. Fixing what it says is separable from deciding what
it should be for.

Gate green: ktlintCheck, detekt, lintDebug.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-22 22:58:16 -05:00
JMR-devandClaude Opus 5 7e7f1301a7 Re-date the audit's testing section, which its own follow-up falsified
R14 / #23, R15 / #24. "On testing these" proposed a plan; the twelve fixes then
executed it, so the section describes a state that no longer exists. It is
re-dated rather than deleted -- the reasoning is why the test stack looks the
way it does -- with each stale claim marked where it stands.

R15 / #24, four statements, each checked against this checkout rather than
against another document:

- "exactly one dependency, testImplementation(libs.junit)". There are five:
  junit, robolectric, androidx.work.testing, the Compose BOM platform and
  compose-ui-test-junit4.
- "a testOptions { unitTests.isIncludeAndroidResources = true } block, which
  this module does not currently have at all". app/build.gradle.kts:105-110.
- "work-testing, compose-ui-test-junit4 and espresso-core ... have zero users."
  By import, work-testing has seven files under app/src/test and
  androidx.compose.ui.test has one. espresso-core really is still at zero, so
  that third is kept as the only part still standing.
- The preamble's "OutputPublisher, both ViewModels, both Workers and
  MainActivity have no JVM unit tests at all". 25 JVM test files were added over
  that set, 180 tests to 257.

That last one is also the derivation of CLAUDE.md's ~31% coverage figure, so the
reasoning is kept verbatim and only its tense and scope are fixed: the ~31% is
what those ~1,200 untested lines produced at 903b43c, it predates the new tests,
and jacoco has not been re-run. The figure itself is deliberately not touched
here -- re-measuring it and updating all three sites together is its own change.

R14 / #23. The audit asserted in two places that instrumented tests cannot run
on this host, and recorded the opposite in a third. 22c7914 is merged and
tools/local-emulator/run-e2e.sh runs API 33-36 locally, so the section's
impossibility argument for "pure seams plus Robolectric" is restated on the
grounds that survive -- speed and determinism, which is the weaker claim and
worth making honestly. D6's "it is an instrumented test, so it runs on CI, not
locally" gets the same treatment, and went further the other way: the
StateRestorationTester test that entry asked for is AppRootRestorationTest,
which runs under Robolectric on :app:testDebugUnitTest. The API 37 rule is
explicitly left standing -- that image is broken and the Pixel check before each
release is unaffected.

No as-found body was rewritten; D6's note is appended to its "Fix direction and
test" guidance, not to its description of the defect. Gate green: ktlintCheck,
detekt, lintDebug.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-22 22:57:01 -05:00
JMR-devandClaude Opus 5 9f0bc9d19b Correct the defect audit's status metadata, which went stale in hours
R3 / #12, R12 / #21, R13 / #22. Three status claims in the audit were false
against `main` at 18c53a3. The audit is read as the work queue for the ticket
phase, so each correction is stated in the document rather than made quietly --
a reader who believed the old claim needs to see that it changed.

R3 / #12. D5 and D7 were marked `open` and "in progress on
fix/space-proxy-and-notification". Both merged hours before: b86df47 (D5) and
c2e6344 (D7) are ancestors of 18c53a3 (`git merge-base --is-ancestor`), and so
is the branch. Anyone working the table would have re-implemented merged work.
The header count was also wrong in its own arithmetic -- "ten fixed, two in
progress, one parked" covers thirteen of sixteen entries, dropping D12, D15 and
D16. It now states four numbers that sum, and says the Fix column tracks `main`
at a named commit so the next reader knows what it is relative to.

R12 / #21. "Where the fixes live" sent the reader to `feat/defect-fixes-base`,
for which `git show-ref` finds nothing -- no local ref, no remote, deleted when
it merged -- and quoted 242 JVM tests. A real run on this branch gives 257 / 0 /
0 / 0 across 34 classes; the 15 missing are UnknownInputSizeTest, SpaceCheckTest
and ProgressNotificationTest. The replacement names `main`, anchors the total to
this commit and gives the command to re-derive it, since the number is only as
fresh as the document.

The same paragraph's "API 33-36 now run locally, 49 tests each" is anchored to
22c7914, the commit that measured it: androidTest is 57 `@Test` on `main`, and
an unanchored total invites a reader to mistake drift for breakage. The
surviving invariant -- 0 failures, 0 errors, 2 skipped, same total at every
level and on the Pixel -- is stated instead.

R13 / #22. D11 was marked `merged`, but 7db3200's own body says one of its four
rows was deliberately skipped: "Not touched: OutputPublisher's hasSpaceFor
KDoc". Still true -- nothing in OutputPublisher.kt mentions D1 -- and that row
is the one place a reader of the code would learn the parked defect exists. The
summary row now says "less the OutputPublisher KDoc row -- held with D1", with
the reasoning at the entry.

The as-found bodies are untouched throughout, including D11's own item table:
corrections are carried as marked editorial notes beside them, the pattern D1's
entry already uses. Gate green: ktlintCheck, detekt, lintDebug.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-22 22:54:33 -05:00