Repository navigation
feat(audio): show how often a sound has been reused while picking one - #8963
Merged
Merged
Conversation
hm21
marked this pull request as ready for review
September 9, 2026 12:47
hm21
requested review from
rabble and
realmeylisdev
and removed request for
a team
September 9, 2026 12:47
This comment has been minimized.
This comment has been minimized.
rabble
approved these changes
Sep 9, 2026
rabble
left a comment
Member
There was a problem hiding this comment.
Reviewed commit 73a12ca. No blocking findings.
Checked the batched query through the Nostr client's filtering/deduplication, attribution ID mapping, picker loading behavior, and removal of the unused widget/localization keys. The documented 500-event batch cap remains a limitation: these are capped reuse counts, not an unrestricted aggregate.
Validation: 492 focused app tests and all 71 sounds_repository tests passed; repository coverage is 100%. Changed app-file analysis and package analysis both passed. No device or live-relay test was performed.
hm21
force-pushed
the
feat/6931-sound-reuse-counts
branch
from
September 9, 2026 17:05
73a12ca to
939cb31
Compare
This comment has been minimized.
This comment has been minimized.
The count path already existed — SoundsRepository.fetchVideosUsingSoundCount, soundUsageCountProvider, and the sound detail header — but a creator choosing a sound could not see reuse anywhere before opening each one. The audio picker's Community tab now reads "00:06 . Artist . 12 videos" per row, using the same localized plural key the detail header uses. A count of zero or unknown renders nothing, so an unused sound reads the same as one whose count has not arrived. soundUsageCountProvider is a NIP-45 COUNT per sound, so watching it per row is the unbounded fan-out #6726 rules out. Instead fetchVideosUsingSoundCounts issues one queryEvents for the whole list and tallies e tags client-side: one relay round trip regardless of row count, matching the same #e predicate the relay applies, so a batched entry equals what the single-sound call reports. trendingSoundUsageCountsProvider derives its id set from trendingSoundsProvider rather than taking a list argument, which would break Riverpod caching and refetch on every rebuild. Ids come from attributionEventId, so bundled assets and unpublished imports are skipped and a list of only those never reaches the repository. Loading and failure are non-blocking: the sheet reads .value, so rows render immediately and simply carry no count. The known trade, documented on the method: limit caps the video events for the whole batch, so a batch whose true total exceeds 500 under-reports. NIP-45 cannot return per-filter counts and there is no funnelcake sound-count endpoint, so this was the only route to a single round trip. SoundTile is deleted along with its orphan test and the four ARB keys only it read. It had no production caller, duplicated AudioListTile, and carried raw TextStyle and raw Icons debt; wiring it in would have meant replacing the tile that already owns selection, the playing indicator and the E2E semantics anchors. The raw_textstyle and raw_icons baselines are regenerated to lock that reduction in the same change.
The committed provider hash was stale against the final edit of sounds_providers.dart, which CI's Generated Files job caught.
hm21
force-pushed
the
feat/6931-sound-reuse-counts
branch
from
September 9, 2026 17:19
939cb31 to
f7165bc
Compare
Mobile PR PreviewPreview refreshed for Last refresh:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
The count path already existed —
SoundsRepository.fetchVideosUsingSoundCount,soundUsageCountProvider, and theSoundDetailScreenheader — but a creator comparing reusable sounds could not see reuse anywhere without opening each one.The audio picker's Community tab (
AudioSelectionBottomSheet, the Nostr/trending list) now reads00:06 ∙ Artist ∙ 12 videosper row, using the existing localized plural keysoundVideoCount— already translated in all 22 locales, and the same key the detail header uses. A count of0or unknown renders nothing, so an unused sound reads the same as one whose count has not arrived yet.Avoiding the per-row relay fan-out
soundUsageCountProvideris a NIP-45 COUNT per sound; watching it per row is exactly the pattern #6726 rules out. Instead:SoundsRepository.fetchVideosUsingSoundCounts(Iterable<String>)issues onequeryEventswithFilter(kinds: [34236], e: [all ids], limit: 500)and talliesetags client-side. One relay round trip for the whole list, regardless of row count. Because it matches the same#epredicate the relay applies, a batched entry equals whatfetchVideosUsingSoundCountreports for that sound.trendingSoundUsageCountsProviderderives its id set fromtrendingSoundsProvider, so there is no list-valued family key — aListargument would break Riverpod caching and refetch on every rebuild. Riverpod caches the result and re-runs only when trending sounds change.AudioEvent.attributionEventId, so bundled assets and unpublished imports are skipped, and a list made only of those never touches the repository at all..value, so rows render immediately and simply carry no counts if the query is pending or fails. That matchesSoundDetailScreen's existingusageCountAsync.value ?? 0.The known trade, documented on the method:
limitcaps the video events for the whole batch, so a batch whose true total exceeds 500 under-reports. NIP-45 cannot return per-filter counts and there is no funnelcake sound-count endpoint, so this was the only route to a single round trip. Today's reuse volumes are well under it; if it ever becomes visible the fix is a server-side count endpoint, not a larger limit — these are full video events at ~1–2 KB each.SoundTileis deletedThe issue's fourth acceptance criterion pointed at
SoundTile's existingvideoCountparameter. It has no production caller, duplicatesAudioListTile, and carries raw-TextStyleand raw-Iconsdebt; wiring it into the picker would have meant replacing the tile that already owns selection, the playing indicator and the E2E semantics anchors. So it and its orphan test are removed, along with the four ARB keys only it read (soundUntitled,soundStopPreview,soundPreviewSemanticLabel,soundViewDetailsSemanticLabel) across all 23 ARB files. Theraw_textstyleandraw_iconsbaselines are regenerated in the same change to lock that reduction in, and thehardcoded_visible_strings_testguard is re-pointed ataudio_list_tile.dart.Related Issue: Closes #6931
Out of Scope
SavedSoundsBloc, not Riverpod, so counts there need a second batched query plus a Cubit and a Riverpod-dependency bridge — a separate, larger change that would fit naturally alongside feat(audio): add creator sound reuse analytics #6726's creator analytics, which needs the same shape.Verification
dart formaton the changed files, thendart format --output=none --set-exit-if-changed→34 files (0 changed).flutter analyze lib test integration_test→ No issues found.cd mobile/packages/sounds_repository && flutter test --coverage→ 71 passed, coverage 100.00 % (177/177). That package's floor is 100.flutter test test/providers/sounds_providers_test.dart test/widgets/video_editor/audio_editor/ test/screens/sound_detail_screen_test.dart test/l10n/ test/widgets/library/→ 492 passed, includingarb_consistency_test.Re-verified after rebasing onto
mainat 3be4f0c (which now carries the Flutter 3.47.2 upgrade from #8915). The only rebase conflict wasmobile/test/widgets/sound_tile_test.dart, which #8915 had reformatted and this branch deletes along withSoundTile— resolved by taking the deletion, since nothing references the widget any more. On the new base: format clean,flutter analyze lib test integration_testclean, the touched app tests and the fullsounds_repositorysuite pass, and that package still measures 100.00 % (306/306).Type of Change