Search-tab top-picks discovery list#284
Open
jubishop wants to merge 51 commits into
Open
Conversation
Implements the search-tab recommendation surface from `docs/initiatives/search-recommendations.md`. SearchViewModel now owns a `SearchRecommendationCollector` that ingests iTunes search and trending rankings, reconciles them against the DB by feed URL + iTunes ID, drops subscribed podcasts, then fans out RSS fetches + embeddings + similarity scoring for the first 25 unsubscribed podcasts. A compact banner above the results grid/list surfaces the loading/loaded state with the doc-specified copy; tapping the loaded banner pushes a search-specific discovery list backed by an ephemeral `SearchDiscoveryListView`. The list uses fixed score-desc / pubDate-desc sort, hides the standard sort toolbar, and routes row actions through the existing ManagingEpisodes pipeline, removing each entry from the collector after a successful materialization. The collector splits its cache into a long-lived per-feed shared cache (reused across trending chip switches) and a query-scoped overlay for typed search that's torn down on each new query. RSS fan-out is capped at 8 in-flight downloads; teardown cancels every in-flight `DownloadTask`. Fixes #280 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ctory Make `SearchRecommendationCollector` a `Container` factory with `.cached` scope so the `.container` test trait gives each test a fresh instance — no `.serialized` needed. The init is now `fileprivate` to force factory construction. SearchView and SearchDiscoveryListView read the collector through `@DynamicInjected`, replacing the prior SwiftUI environment plumbing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
jubishop
commented
May 18, 2026
- Drop unneeded explanatory comments in Repo.episodesMatching and RecommendationEngine.hasScoringContext. - Inline DownloadManager construction in SearchRecommendationCollector; remove the searchDiscoveryDownloadManager container factory. - Extract SearchDiscoveryActionsViewModel into its own file. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
jubishop
commented
May 18, 2026
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
jubishop
commented
May 19, 2026
SearchViewModel now creates the collector directly instead of resolving it from a .cached Container factory. SearchView publishes it via .environment(...) so SearchDiscoveryListView can pick it up with @Environment. teardown() and the disappear()-time call are removed; the instance now lives for the view model's lifetime. Also addresses review-comment cleanup: Episode candidate gate moved to a private static helper on the collector, performAfterMaterialize uses Container.shared directly instead of binding @DynamicInjected projections to temp vars, and a handful of doc comments are trimmed or reference constants by name. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ediaGUIDs Replace the `sharedPodcastCache` + `typedSearchOverlay.localEntries` split with two top-level dicts: `permanent` (entries ever referenced by a loaded trending chip) and `temporary` (entries referenced only by the current typed-search ranking). A feedURL that first appears in a search and then in a trending chip gets *promoted* — the existing entry moves from `temporary` to `permanent`, so we never kick off a duplicate `DownloadTask` for the same URL. On a search-query change, cancel + `temporary.removeAll()` is O(1), and we strip those URLs from the pending drain queue. Make `CachedPodcastEntry` `@Observable` (with `fetchToken` marked `@ObservationIgnored`) so status / scoredEpisodes mutations propagate naturally. Convert `visiblePicks` and `bannerState` to computed properties that read the dicts and entry observation tracks the dependencies. Delete `refreshOutputs()` and every call site for it — SwiftUI invalidation handles all the prior recompute points (`applyReconciledRanking`, `processFeedURL`, `setActiveSource`, `removePick`). Drop `removedMediaGUIDs`. `removePick(feedURL:mediaGUID:)` now mutates the owning entry's `scoredEpisodes` array directly; the entry is the single source of truth, and observation propagates the mutation. The caller (`SearchDiscoveryActionsViewModel.performAfterMaterialize`) passes the feedURL alongside the mediaGUID for O(1) entry lookup. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ce-specific picks, and cold engine handling
Introduce three major improvements to the recommendation pipeline:
**Teardown & Memory Management**: Add `tearDown()` method called when exiting Search tab to cancel in-flight tasks, clear all caches (permanent, temporary, trending index, debouncers), and prevent discovery candidates from lingering in memory indefinitely.
**Source-Specific Picks API**: Decouple `picks(for:)` and `bannerState(for:)` from `activeSource` so pushed discovery lists continue rendering their own source even if SearchView swaps the active source underneath. This enables proper multi-source rendering without interference.
**Cold Engine Deferral**: Implement `awaitScoringContext()` to block the drain loop until the recommendation engine is hydrated. Prevents the pipeline from racing through scoring with a nil cache and prematurely marking entries as `.scored` with no picks. Entry drain loop now waits for engine warmth before processing.
**Typed-Search Query Retention**: Replace `purgeTemporary()` with `pruneTemporary(keeping:)` to preserve temporary entries whose feedURL re-appears across query changes. In-flight RSS and embedding work now carries forward instead of being orphaned, improving UX for overlapping search results.
**Minor Cleanups**:
- Simplify `prefix().map {}` to direct `Array(prefix())` conversions
- Remove redundant whitespace trimming in `pushSearchResultsToCollector`
- Guard against weak self capture failures in drain task setup
…down Fix a race condition where `tearDown()` would cancel the `drainTask`, causing `processFeedURL` child tasks to resume and clear their fetch tokens before the teardown task could cancel the entries. This left active downloads orphaned in the download manager. Now explicitly call `downloadManager.cancelAllDownloads()` before cancelling entries, ensuring the underlying DownloadTasks are terminated even after entries have lost their fetch tokens. This follows the manager's ownership model for active downloads. Add comprehensive test coverage: - `tearDownCancelsInFlightDownload`: Verify that in-flight RSS downloads are properly cancelled when tearDown is called - `typedSearchQueryChangeDoesNotCancelTrendingWork`: Ensure that switching typed-search queries doesn't accidentally cancel work owned by trending sources, validating that only overlay-owned entries are cancelled on query changes
…ng to recommendation engine Add `saveEpisodeInCache` and `unsaveEpisodeFromCache` methods to the ManagingEpisodes protocol to enable dynamic dispatch for cache operations in episode context menus. Enhance SearchRecommendationCollector with robust handling for unavailable scoring contexts: - Introduce `entryFeedURL` field in ScoredEpisode to handle cases where parsed feed URLs differ from cache keys - Improve `removePick` logic to search by mediaGUID when direct feedURL lookup misses, enabling reliable pick removal even when RSS-parsed URLs differ from requested URLs - Add `scoringUnavailable` flag and `scoringContextWatcherTask` to prevent indefinite blocking when the recommendation engine cannot build a scoring context (insufficient user signal data) - Modify `awaitScoringContext` to detect unavailability after the first buildContext attempt and return early instead of looping forever - Implement `observeScoringContextWarmth` to re-queue previously-scored entries when scoring context later becomes available - Update banner state logic to surface .hidden when scoring is unavailable - Fix teardown to explicitly cancel debouncer tasks, preventing stale post-teardown actions from repopulating caches - Pass `entryFeedURL` through the pipeline to populate ScoredEpisode records Add comprehensive test coverage for feed URL mismatch handling, dispatch behavior verification, and banner state transitions during unavailable scoring scenarios.
…on race condition Fix a race condition in `awaitScoringContext()` where AppLauncher pre-starts the recommendation engine, causing the bootstrap emit to be missed when `scoringRevision > 0` but `hasScoringContext` is still false. Replace `dropFirst()` with explicit revision checking to detect when the engine has already rebuilt to nil context before the collector subscribes. Changes include: - Add early exit in `awaitScoringContext()` when `scoringRevision > 0` to unblock the drain and allow RSS downloads to proceed - Remove `dropFirst()` from the main scoring revision stream to capture all state changes including bootstrap replays - Update `observeScoringContextWarmth()` to subscribe without `dropFirst()` since engine state can change between caller setup and watcher subscription - Add test case `bannerHidesWhenEnginePreRebuiltToNil()` to verify banner hides correctly when engine rebuilds to nil context before collector starts monitoring
…ment Separate typed-search debouncing from trending-source debouncing to prevent stale queries from firing after user navigates away. Replace per-source debouncers with a shared `typedSearchDebouncer` that cancels prior queries when a new search is recorded, ensuring `foo`'s pipeline doesn't execute after user types `bar`. Add `clearTypedSearchOverlay()` method that fires when leaving typed search (via `setActiveSource`) to release overlay-owned temporary cache and cancel pending debouncer tasks. This prevents memory leaks and orphaned RSS requests for abandoned queries. Replace boolean `scoringUnavailable` with enum `ScoringAvailability` (unknown/ready/unavailable) for clearer state representation. Remove `.embedding` status from `CachedPodcastEntry.Status` as embedding now completes before `fetching` status ends. Fix memory leak in `SearchDiscoveryActionsViewModel.performAfterMaterialize` by capturing `[weak self]` in task and adding guard check to prevent use-after-free on `collector`. Add comprehensive test coverage for typed-search cleanup scenarios: overlay release on source transition, pending debouncer cancellation when leaving search, and stale query pipeline cancellation when recording new queries.
2 tasks
The drain task can be scheduled (banner → .loading) before it has reached engine.start() → observation emit → scheduleCacheRebuild() → cacheDebounce's 5 s sleep registration. If advanceTime runs first, currentTime jumps past the not-yet-scheduled wakeTime and the rebuild never fires — banner stays .loading. Wait for the sleep to register first, then advance. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… reconciliation Replace `entryFeedURL` field in `ScoredEpisode` with defensive re-scheduling logic to handle concurrent entry swaps. Change `pendingDrainQueue` from `[FeedURL]` to `OrderedSet<FeedURL>` for O(1) membership checks and automatic deduplication. Remove whitespace trimming from search query display strings since `searchedText` is already trimmed upstream. Add re-push calls to both search and trending observation callbacks so the collector re-reconciles against the database when podcasts transition to subscribed state, ensuring their picks are properly dropped from the recommendation list. Update `runPipeline` to remove the unused `entryFeedURL` parameter and simplify `ScoredEpisode` initialization. Add comprehensive test coverage for the subscription state transition behavior to prevent regression when subscribing to trending rows.
…ecommendation collector tests
Refactor `performAfterMaterialize` to pass the materialized `PodcastEpisode` directly to action closures instead of just the episode ID, eliminating redundant database lookups across all episode actions (play, queue, cache, rate, mark finished). This reduces database round-trips and improves clarity by working with the full object rather than reconstructing it.
Reorganize the sprawling 1266-line `SearchRecommendationCollectorTests` into focused test suites by responsibility:
- `SearchRecommendationCollectorIngestTests`: reconciliation, debouncing, scoring
- `SearchRecommendationCollectorRemovalTests`: post-action pick removal and ManagingEpisodes dispatch
- `SearchRecommendationCollectorTeardownTests`: lifecycle cleanup and cancellation
- `SearchRecommendationCollectorScoringAvailabilityTests`: engine state and scoring context handling
- `SearchRecommendationCollectorTypedSearchTests`: typed-search overlay lifecycle and debouncer behavior
- `SearchRecommendationCollectorTestHelpers`: shared test utilities (RSS generation, engine priming, fake session access)
Consolidate recommendation banner source logic: remove optional wrapper and guarantee valid source from `recommendationBannerSource` property. Improve search result banner pluralization ("Top pick" vs "Top X picks"). Implement missing typed-search overlay cleanup: when a search query returns empty results or errors, push the empty result set to the collector to discard the prior overlay and cancel any in-flight work. Use `chain()` from Algorithms to simplify repeated iterations across permanent/temporary cache entries in `removePick`.
jubishop
commented
May 25, 2026
Remove verbose multi-line comments explaining implementation details and replace with concise single-line summaries. Consolidate related comment blocks and remove redundant explanations that repeat information already clear from code context. - Shorten SearchDiscoveryActionsViewModel class comment from 4 lines to 1 - Condense SearchRecommendationCollector header from ~15 lines to 2 lines explaining cache lifecycle - Simplify tunable parameter comments, removing implementation rationale - Trim verbose state variable comments explaining cache ownership and lifecycle - Tighten method documentation throughout, focusing on intent over mechanism - Remove redundant comments in test files that restate obvious code behavior - Clean up tear-down and debouncer cancellation comments in both source and tests - Consolidate feed URL reconciliation and candidate filtering comments - Reduce test setup explanations that don't add value to understanding assertions
…tion and add post-action hook Add `didPerformAction` default protocol method to `ManagingEpisodes` to support post-success bookkeeping in row-action handlers. Call this hook after play, queue, cache, rating, and finish operations complete successfully. This allows `SearchDiscoveryActionsViewModel` to remove picks from the collector after episode actions succeed without duplicating action logic. Pass `SearchDiscoveryActionsViewModel` through the navigation destination and init it once in `SearchViewModel` so the same instance survives view re-renders. Update `SearchDiscoveryListView` to accept and use the passed actions view model instead of creating a new one per render. Simplify `SearchDiscoveryActionsViewModel` to only override the post-action hook; all action implementations now come from protocol defaults. Fix race condition in `SearchRecommendationCollector` startup where scoring context could become ready between the initial read and subscription setup; re-check availability before subscribing to avoid briefly hiding the banner. Remove environment passing of collector from `SearchView`. Add end-to-end test verifying pick removal after queue action through actions view model.
…revent duplicate podcasts on feed URL changes Thread the iTunes podcast ID through the entire recommendation collection and caching pipeline to enable deduplication by iTunes ID when RSS feeds publish different atom:self or itunes:new-feed-url values. This fixes a regression where podcasts fetched via iTunes lookup would create duplicate rows when the canonical feed URL differed from the RSS-published self URL. Changes include: - Add `reconciledITunesIDs` tracking in `recordSourcePodcasts` to preserve iTunes ID mappings alongside feed URLs - Thread `iTunesID` parameter through `applyReconciledRanking`, `runPipeline`, and `filteredCandidates` methods - Update `CachedPodcastEntry` to store and pass through the iTunes ID to downstream upsert operations - Pass `iTunesID` when calling `toUnsavedPodcast()` so the database layer can deduplicate by both feed URL and iTunes ID - Elevate error handling in `filteredCandidates` by removing silent error swallowing and letting errors propagate - Update search overlay placeholder text to better reflect the "worked through all picks" state - Quote search queries in discovery list titles for clarity - Add preview container registration in SearchView to ensure fake session is injected before ViewModel construction - Add regression test covering iTunes ID deduplication across atom:self URL shifts
jubishop
commented
May 26, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
- Navigation.showSearchDiscovery() now dismisses any presented sheet before pushing, matching the surrounding show* helpers. - SearchRecommendationCollector.recordSourcePodcasts() bumps a typed- search generation counter; reconcileAndIngest re-checks it after the observation await and bails before clobbering a newer overlay. - The scoring-context watcher subscribes to $scoringRevision.stream() and stays armed for the collector's lifetime, so engine close→open cycles continue to re-queue entries that scored empty in between. - SearchViewModel.restartObservationForSearchResults captures the search term once so a late observation emission can't reattribute its picks to a newer query. - RecommendationEngine.scoringContextReady is now private; readiness is observed via hasScoringContext / $scoringRevision.stream(). - Tests: scoring-availability predicate now resolves the feed session once via H.session; typed-search overlay-replacement test asserts the first query's RSS request was cancelled before signalling its hanging semaphore. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The latch became redundant after the collector switched to observing `$scoringRevision.stream()` for readiness transitions — nothing was awaiting `wait()` anymore. `hasScoringContext` now reads `cache()` directly so the cache is the single source of truth, and the cache rebuild no longer has to mirror its state into a second piece of state. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
jubishop
commented
May 26, 2026
jubishop
commented
May 26, 2026
AppLauncher already starts the engine and start() is idempotent, so the collector's call was dead. Update the scoring-unavailable test to start the engine itself, matching production where AppLauncher does it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
It's a private helper called only from setActiveSource; group it under a dedicated Typed-Search Overlay section so the Public API block contains only public methods. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…scoping Adds EpisodeQueryTests coverage for the new episodesMatching(podcastID:guids:mediaURLs:): empty-input early return, guid-only and mediaURL-only filtering, the OR union when both lists are populated, dedupe when a row matches by both criteria, scoping to the target podcast when a guid is shared across podcasts, and the no-matches case. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@PodHaven/Views/Search/Models/SearchRecommendationCollector.swift`:
- Around line 230-234: The removePick implementation can leave an entry in the
.scored state with an empty scoredEpisodes array which later gets re-queued by
handleScoringContextBecameAvailable; change removePick (in
SearchRecommendationCollector.removePick) to mark the entry as non-retryable
when the last episode is removed (either by setting entry.state to a distinct
non-retryable enum case such as .userRemovedAll or setting a boolean like
entry.userRemovedAll = true), and update handleScoringContextBecameAvailable to
skip entries flagged as user-removed (or with the new non-retryable state) when
deciding what to re-queue; ensure pickIndex and any persistence/serialization of
entry state are updated to include this new flag/state so user-removed entries
never get retried on scoring-context reopen.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 43a6dca1-42af-438a-bf04-7d33b452dcbb
📒 Files selected for processing (2)
PodHaven/Views/Search/Models/SearchRecommendationCollector.swiftPodHavenTests/ViewModelTests/SearchRecommendationCollectorTests/SearchRecommendationCollectorScoringAvailabilityTests.swift
…ring close → open removePick used to leave a fully-dismissed entry in `.scored` with empty `scoredEpisodes`, which the watcher's close → open recovery couldn't tell apart from a cold-engine "scored without picks" entry — so the next scoring-context cycle re-fetched RSS and brought the dismissed pick back. Add a terminal `.exhausted` status set by `removePick` when the last pick is dismissed; teach `scheduleDrain`, `dequeueNext`, the `processFeedURL` race-check, and `bannerState` to skip it. The recovery loop in `handleScoringContextBecameAvailable` still re-queues only `.scored && empty`, so user-dismissed entries stay dismissed. Regression covered by `removedPickSurvivesScoringContextCycle`, which drives a real engine close → open by unrating then re-rating signal episodes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…h overlay lifecycle Replace enum source identity with stable ID for SwiftUI view persistence, preventing unnecessary view teardowns when internal source properties change. Add `stableID` computed property to `SearchRecommendationCollector.Source` that generates consistent identifiers across enum variants. Rename `tearDown()` to `reset()` throughout codebase to better reflect that the collector is reusable after clearing caches and cancelling tasks. Bump `typedSearchGeneration` in `clearTypedSearchOverlay()` to invalidate stale reconcile operations that complete after the overlay is cleared, preventing orphaned state from re-creating the overlay. Add generation guard check in scoring pipeline to skip writing results for detached entries, preventing pickIndex corruption when entries are purged mid-pipeline. Cache `currentEntry` lookup to avoid redundant calls. Move `.searchable` modifier to root content in SearchView to prevent pushed destinations from inheriting the search field, avoiding state invalidation when editing queries from pushed views. Improve grammar in banner copy to use singular "Top pick" when count equals 1. Extract `rssXML` helper to `SearchRecommendationCollectorTestHelpers` for test reuse. Update all test names and documentation from "teardown" to "reset" terminology. Add new test verifying stale reconcile bailout after typed-search clear.
…nciliation Prevent duplicate picks from appearing when multiple iTunes slots bridge to the same saved podcast via iTunes ID. Add a `seenCanonicalFeedURLs` set to track and filter out duplicate canonical feed URLs during reconciliation, ensuring each unique podcast appears only once in the results. Additionally, refactor SearchView by moving `resultsGrid` and `resultsList` computed properties earlier in the file for better code organization and readability. Include comprehensive test case verifying that two iTunes rows with different slot URLs but the same iTunes ID properly deduplicate to a single canonical feed URL, preventing duplicate picks and banner counts.
…ation with constant reference Update test cases to use `SearchRecommendationCollector.stableSourceDebounce` instead of hardcoded `.seconds(1)` duration. This makes tests more maintainable and ensures they adapt automatically if the debounce timing is adjusted. Also update test descriptions and comments to refer to "stable-source debounce" terminology instead of "1 s" to improve clarity and consistency. Update documentation to clarify that debounce timing and other caps are tunable constants defined near the collector, removing specific hardcoded values from the conceptual design doc so it reflects the source of truth in code.
…e download task cancellation race Fix a critical issue where RSS feeds were being fetched and discarded when the recommendation engine had not yet finished bootstrapping with scoring context. The drain now properly blocks on `awaitScoringContext()` instead of proceeding immediately, preventing unnecessary network requests for new users browsing trending chips. Additionally, handle a race condition in `processFeedURL()` between setting `entry.status = .fetching` and assigning `entry.fetchToken = downloadTask`. If a purge (typed-overlay clear or pruneTemporary) occurs during this window, `entry.cancel()` reads a nil fetchToken and cannot reach the just-acquired download task. Add a guard after `addURL()` to detect this scenario and cancel the task ourselves, ensuring the RSS fetch doesn't run to completion only to have its result discarded. Simplify `awaitScoringContext()` by removing the redundant re-check after setting `.unavailable`, since the watcher will emit the next revision and flip availability back to `.ready` if context becomes available. Update `stableID` to include both `genreID` and `title` to prevent collisions between semantically-different trending values that share a genreID. Remove unnecessary `syncCollectorActiveSource()` call from `SearchViewModel.appear()` since the active source is already set via `showTrendingSection()`. Add comprehensive test coverage for the scoring availability behavior and stress-test the addURL cancellation race with multiple concurrent feeds.
…om resurrecting state after reset Add Task.isCancelled check after firstObservationEmission to bail out early if the task was cancelled during the observation suspension. This prevents a stale debouncer-triggered reconciliation from recreating the typed overlay, rebuilding the temporary cache, and queuing RSS fetches after the collector has been reset and torn down. The firstObservationEmission method swallows CancellationError and returns an empty array, which would otherwise allow the reconciliation to continue executing applyReconciledRanking even though the task should be cancelled. The new guard clause ensures we exit immediately when cancellation is detected, preserving the teardown state and preventing unwanted side effects from stale operations.
…load cancellation logic Extract the search query debounce duration to a named static constant in SearchViewModel to eliminate magic numbers and improve maintainability. Update all test references to use the new constant for consistency. Refine the download cancellation strategy in SearchRecommendationCollector to only cancel URLs from the current snapshot instead of all active downloads. This prevents inadvertently cancelling downloads that a freshly-registered `recordSourcePodcasts` initiated before the unstructured Task runs, since `downloadManager` is reused across resets. Add re-scheduling logic for entries left in mid-fetch state to ensure they aren't stranded when replaced by a purge operation. Simplify the `discoveryListTitle` computed property to always return `trending.title` instead of conditionally falling back to "Top picks" when genreID is nil, as the title itself now handles this distinction.
…on with AsyncStream for drain queue Replace the manual continuation-based coordination between `scheduleDrain()` and `runDrainLoop()` with an `AsyncStream<FeedURL>` that provides a cleaner, more structured approach to work distribution. The new design uses a single dispatcher that consumes the stream and fans work into a bounded task group, with automatic deduplication via `queued` (backlog) and `inFlight` (active work) sets. Key changes: - Replace `pendingDrainQueue` and `drainContinuation` with `queueContinuation` and an `AsyncStream` - Rename `pendingDrainQueue` to `queued` for clarity about its role in deduplication - Extract `shouldDrain()` to centralize drain eligibility logic and add `.failed` status check to prevent re-fetching failed feeds - Add `isFeedURLDetached()` to detect when purges swap or drop entries, letting the embed loop bail early - Refactor `runDrainLoop()` to iterate directly over the stream with inline task group management instead of separate `dequeueNext()` and `waitForWork()` methods - Add `pickCount()` helper to optimize banner state calculation (skip sorting when only count is needed) - Add comprehensive test `failedFeedNotRefetchedOnReRecord()` to verify failed feeds aren't re-fetched on source re-record - Pass `isDetached` closure to `runPipeline()` so embedding can abort work for detached entries early
…roup for efficient work distribution Replace the manual bounded task group implementation with Swift's native `withDiscardingTaskGroup` to simplify work distribution and improve memory efficiency. The discarding group automatically reaps completed tasks immediately rather than accumulating records, eliminating the need for manual `active` counter management and explicit `group.next()` calls. Update the accompanying documentation to clarify that concurrent work is bounded by downstream components (DownloadManager and ContextualEmbedding actor) rather than a fixed task group limit. This architectural change simplifies the code while maintaining the same effective concurrency behavior through natural bottlenecks in the pipeline.
… NoOp to noOp Update the AppDB constant reference to use the correct camelCase naming convention. Change `AppDB.NoOp` to `AppDB.noOp` to align with Swift naming standards where constants use camelCase rather than PascalCase. This ensures consistency with the actual API and prevents potential runtime errors due to incorrect constant references.
…tead of deriving it Replace implicit source derivation with explicit parameter passing to the banner strip and copy methods. This change converts `recommendationBannerSource`, `loadingBannerCopy`, and `loadedBannerCopy` from computed properties into methods that accept the source as a parameter, making the data flow more explicit and testable. Update `recommendationBanner` to conditionally render only when an active source exists, and pass the source directly to all banner-related methods. This eliminates the need for the view to independently determine which source is active, reducing coupling and making the recommendation banner behavior more predictable and easier to maintain.
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
2 participants
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.
Summary
SearchRecommendationCollector, owned directly bySearchViewModelfor the lifetime of the Search tab. Reconciles iTunes search/trending rankings against the DB, drops subscribed podcasts, then fans out RSS+embed+score for the first 25 unsubscribed feeds.docs/initiatives/search-recommendations.md; tap pushes a newSearchDiscoveryListViewonto the search navigation stack with score-desc / pubDate-desc sort and a hidden sort toolbar. The banner'sSourceand the sharedSearchDiscoveryActionsViewModelare threaded to the list through a newNavigation.Destination.searchDiscoverycase (vianavigation.showSearchDiscovery(...)), so the pushed list stays pinned to the source it was created with even if the active search surface changes.Why
Implements the v1 plan in
docs/initiatives/search-recommendations.mdand meets the acceptance criteria in #280.Test plan
SearchRecommendationCollectorTestscovers happy path, stable-source debounce, subscribed exclusion via iTunes ID reconciliation, candidate-gate filtering for materialized rated rows, score floor, post-action removal, typed-search overlay replacement, and cross-category shared-cache reuse.SearchViewModelTestscontinue to pass.xcodebuild test).Known limitations (v1)
Fixes #280
🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes