Hide Shorts in the subscription feed (on by default) #31

Merged
kayos merged 14 commits from feat/hide-shorts-feed into main 2026-10-22 14:39:48 -07:00

14 commits

Author SHA1 Message Date
3c2f4d485f test: the screens take the Hide Shorts decision from the tested functions
All checks were successful
build-apk / build-and-publish (push) Successful in 26m54s
From the red team over ab344c80b. Three wiring mutants survived the 31 shipped
tests: the Subs tab no longer calling subsTabRows, the search screen calling
the boolean form with false, and the Subs tab keeping the fixed empty text.
The fix round declined them as needing a Compose UI test; a plain JVM test that
reads the two screens' source kills each of them (the red team measured each
mutant failing exactly one test, and the clean tree passing). Run here in the
CI image: testDebugUnitTest BUILD SUCCESSFUL, ScreenWiringTest 2 tests, 0
failures.

Correction to ab344c80b: its message says the workflow was not run on Forgejo
because it only runs on main. That is not the reason. build.yml also runs on
workflow_dispatch, which takes any ref, and the publish step runs on any
workflow_dispatch whatever the ref (the step's if: condition), so dispatching
this branch to test CI would have published an APK built from the branch. The
change was not tested on Forgejo for that reason.
2026-10-22 14:39:30 -07:00
ab344c80bc ci: run the Rust and JVM unit tests before building the APK
The build-apk job ran only ./gradlew :strawApp:assembleDebug, so no
unit test ran before an APK was signed and published. With the Shorts
check planted to never match, that command still built successfully.

Two steps are added after "Auto versionCode from commit count" and
before "Assemble debug APK":

- Rust unit tests (strawcore): cargo test -p strawcore --lib, in
  straw/rust.
- JVM unit tests: ./gradlew :strawApp:testDebugUnitTest, in straw.

Both run under the same ionice/nice as the build step and with the same
CARGO_TARGET_DIR; neither has a secret in its environment. Nothing else
in the workflow changed: same triggers, same build, signer check and
publish steps and conditions. Lint is not added.

Run by hand in the straw-build image, in the workflow's order:
cargo test 11 passed, rc 0; testDebugUnitTest 31 passed, rc 0;
assembleDebug rc 0. With the Shorts check planted to never match, the
JVM step exited 1 (6 of 31 failed); with the Rust parser planted to
never mark a Short, the Rust step exited 101 (5 of 11 failed).

Not run on Forgejo: the workflow only runs on main.
2026-10-22 12:58:53 -07:00
09a8319a67 content filter, feed: two wordings that said more than is true
- The settings form of applyContentFilters said "a screen calls this
  one". The channel screen still passes its switch itself; the comment
  now names the two callers that use it.
- A parser test was named "in any order" but checks both orders of two
  links and one order of three; the name no longer says "any".

Comment and test name only; no code changes.
2026-10-22 12:51:07 -07:00
d90696a2bb content filter: drop a channel-tab count that did not reproduce
looksLikeShort's comment said one channel's Videos tab held "0 of 30"
Shorts. A review could not reproduce that count, so the number is
removed.

The comment now says only what holds: search with the Videos filter
returned no Shorts (0 of 79 results across 4 queries, unchanged);
channel pages list the Videos tab, whose items strawcore-core also
gives watch?v= URLs (read in its channel.rs), so this check cannot see
a Short there; whether that tab ever lists one was not measured.
2026-10-22 12:47:31 -07:00
5a67e9ea65 feed: alternate links that disagree mark nothing; pin three link shapes
Three parser inputs were not pinned by a test, and changing how each is
handled left all 8 strawcore tests passing:

- An entity-escaped href (https:&#47;&#47;...shorts&#47;<id>). It is
  unescaped as XML and marks a Short. Now tested.
- A shorts link whose id differs from the entry's only in letter case.
  It does not mark a Short: the id is compared exactly, as entry_url's
  comment says. Now tested. Measured 2026-10-22: YouTube's oEmbed
  endpoint answered 200 for E_MGy41IYVw, 404 for e_mgy41iyvw and 400
  for E_MGY41IYVW.
- Two alternate links in one entry. The last one used to win, so
  "watch, then shorts" hid the video and "shorts, then watch" did not.

Decision for the last case: an entry decides from its alternate links
only when they are all the same href (agreed_alternate). If they
disagree it decides from none and keeps its watch URL, in either
order. A self-contradicting feed can then at worst show a Short, never
hide a regular upload. In the 326 channel feeds captured for this
change on 2026-10-22 (4,647 entries) every entry carried exactly one
link, an alternate one, so this changes nothing for any feed seen.

Tests: the escaped href, the case variant, and four entries: shorts
then watch, watch then shorts, shorts twice then watch (all watch
URLs), and the same shorts link twice (a Short).
2026-10-22 12:47:06 -07:00
3daa7a9ed0 feed cache: pin the keys save and clear write to
save() and clear() went through PrefsWriter, which launches on the
app-wide coroutine scope that only the running app sets up, so no unit
test could call them. With save() writing the old cache_v1 key, or
clear() removing it instead of cache_v2, all 21 unit tests still passed.

FeedCacheStore now takes its writer as a constructor argument. The app
paths are unchanged: both existing constructors pass a PrefsWriter on
the same preferences, as before. A test passes a writer that edits the
in-memory preferences at once.

Tests: save writes cache_v2, writes nothing under cache_v1, and loads
back what it saved; clear removes cache_v2.
2026-10-22 12:46:45 -07:00
03107ad328 feed: when only hidden Shorts are left, the Subs tab says so
With Hide Shorts on (now the default), a feed whose every item is a
Short, or whose only unwatched items are Shorts, left the Subs tab
saying "All caught up — nothing unwatched." That was not true: there
were unwatched items, and Hide Shorts was hiding them.

subsTabRows now also returns how many items Hide Shorts removed from
what Hide watched left, and the line to show when nothing is left:
"Nothing to show but Shorts, and Hide Shorts is on in Settings." when
any Shorts were hidden, and the old line otherwise. The Subs tab shows
that line in place of the fixed text.

Tests: a feed of only Shorts, watched rows hidden with only Shorts
left, and an all-watched feed that still says all caught up.

No log line for the hidden count: none of the files involved log
anything today, and adding logging is not part of this change.
2026-10-22 12:46:38 -07:00
a94a95e0f1 feed, search: decide Hide Shorts in tested code, not in the composables
The Subs tab (StrawHome) and the search screen each passed the Hide
Shorts switch to applyContentFilters themselves. Those lines were the
only thing hiding Shorts there, and no test reached them: with either
one changed to pass false, all 21 unit tests still passed.

Now:
- applyContentFilters has a second form that takes the SettingsStore
  and reads the switch itself. The search screen calls it.
- subsTabRows (feature/feed/SubsTabRows.kt) builds the Subs tab's rows:
  hide watched as before, then that same filter. The Subs tab calls it.
  extractVideoId moved there with it, unchanged.
- The composables still collect the switches and key the remembered
  list on them, so a flipped switch recomputes the list as before.

Tests build a real SettingsStore on in-memory preferences: an untouched
install sees no Shorts on the Subs tab or through the settings form of
the filter, a stored false shows them, and Hide watched neither hides
Shorts nor is read in place of the Shorts switch.

Not covered: a composable that stops calling these functions. That
would need a Compose UI test, which this module does not have.
2026-10-22 12:46:22 -07:00
67a831b8e1 feed: spend the metadata backfill on rows that are shown
The feed backfills views and duration for its first 30 rows, one strawcore
call per row. It counted those 30 before Hide Shorts removed anything, so
with the switch on (now the default) calls went to Shorts that are never
shown and fewer visible rows got numbers. Hidden Shorts are now dropped
before the 30 are counted. Same cap, same number of calls at most.
2026-10-22 08:05:20 -07:00
84f10596d0 feed cache: move to cache_v2 and drop the cache written before /shorts/ URLs
A cache_v1 entry holds the feed as it was before strawcore marked Shorts,
with every Short under a watch?v= URL, so after an update it would paint
those Shorts until each channel happened to be refetched. The store now
reads and writes cache_v2 only, and removes cache_v1 when it is built (it
was the whole feed, about 225 KB per the store's own estimate, sitting in
the same preferences file). The first launch after the update paints
without a cached feed and fetches it.

load() keeps its behaviour; the body moved to an internal load(ttl) so a
JVM test can run it without the Settings singleton.
2026-10-22 08:05:20 -07:00
8ec2185839 content filter: a Short is an item with a /shorts/ URL, nothing else
The title fallback ("#shorts", "#short", "(shorts)") matched 0 of the 33
Shorts in the 7 channel feeds measured on 2026-10-22, and with Hide
Shorts on by default it would hide any long video whose title only
contains "#short", such as "#shortfilm". The subscription feed now gets
/shorts/ URLs from strawcore. Search (Videos filter, 0 of 79 results over
4 queries) and one channel's Videos tab (0 of 30) returned no Shorts, so
the URL check is all that is left to do there.
2026-10-22 08:05:20 -07:00
09637b9cc5 settings: Hide Shorts is on by default
An install that never touched the switch now hides Shorts; the switch
still shows them again. The key stays hide_shorts_v1 and only its default
changes, which moves exactly the untouched installs: setHideShorts is the
only writer of the key, its only caller is the Settings switch, and since
it was added in 06bd17d82 it has skipped the write when the value did not
change. So no install can hold a stored false that came from the old
default; a stored false means the user turned the switch on and off, and
it is kept. Tests pin all three cases (no value, false, true).

SettingsStore takes its SharedPreferences through an internal constructor
so a JVM test can build it; the Context constructor is unchanged for the
app. The Settings description now says what the switch does.
2026-10-22 08:05:20 -07:00
28bc38bb0b build: put junit on the app's unit-test classpath
strawApp/src/test had no test framework, so LogDumpScrubTest was never
compiled or run. With junit added, `./gradlew :strawApp:testDebugUnitTest`
compiles and runs it: 10 tests, 0 failures (run 2026-10-22 in the
straw-build image). CI still runs only assembleDebug; this does not
change the workflow.
2026-10-22 08:02:19 -07:00
1f94be7ef7 feed: keep the Shorts signal the channel RSS feed already carries
A Short's <link rel="alternate"> in the channel feed points at
https://www.youtube.com/shorts/<id>; every other upload points at
/watch?v=<id>. parse_rss threw that link away and built a watch URL for
every entry, so the app's hide-Shorts filter could only guess from titles.

Measured 2026-10-22 on 7 public channel feeds (91 entries, channels with
many, few and no Shorts and two that stream live): the 33 entries with a
/shorts/ link were exactly the entries listed in the channel's Shorts
playlist feed (playlist_id=UUSH...), and none of the 58 /watch entries
were. Running parse_rss over those captured feeds gives the same 33.

parse_rss now gives an entry its /shorts/<id> URL when its alternate link
is exactly that form for its own video id, and the watch URL otherwise.
No extra request: it is the same feed.
2026-10-22 07:57:05 -07:00