fix: stabilize discover/likes races and improve location UX - #30
Merged
Conversation
Keep swiped cards out of Discover (session filter + no HTTP cache), merge optimistic likes/passes so the Likes tab does not flash-remove just-swiped items, and surface popular NCR cities offline while Places is loading. Also drop the floating like/pass action bar (gesture-only deck), tune Android Gradle memory for low-RAM builds, and clear pre-existing analyzer warnings that blocked pre-commit.
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
📝 WalkthroughWalkthroughThe PR updates Android tooling, adds curated popular-city location suggestions, improves swipe and likes state synchronization, replaces discover action buttons with gestures and semantic actions, and adjusts related tests, translations, and generated configuration. ChangesAndroid and tooling configuration
Popular city location search
Swipe and likes state consistency
Maintenance
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
lib/core/controllers/page_state_service.dart (2)
581-588: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftRollback the optimistic transaction when the swipe write fails.
All local caches and Discover state are mutated before
_swipesPort.recordSwipe(). Several supplied callers only log the exception, leaving a failed swipe hidden from Discover and preserved in Likes for the session. Capture a rollback closure/snapshot, restore it incatch, then rethrow the mapped error.As per coding guidelines, “Wrap async calls in try/catch blocks and map errors via
error_mapper.dartand handle witherror_handler.dart.”🤖 Prompt for 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. In `@lib/core/controllers/page_state_service.dart` around lines 581 - 588, Update the swipe flow containing _sessionSwipedPropertyIds, removePropertyFromDiscover, and _swipesPort.recordSwipe to capture the affected local state and wrap the network call in try/catch. On failure, restore Discover and cache/session state from the captured snapshot, map the exception through error_mapper.dart, handle it with error_handler.dart, and rethrow the mapped error so callers receive the failure.Source: Coding guidelines
125-136: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftInvalidate in-flight loads when clearing the user session.
clearSessionData()resets state, but an earlierPageDataLoaderrequest can still complete and write the previous user’s properties back into these observables. Add a session/request generation token, invalidate it here, and discard responses from older generations.🤖 Prompt for 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. In `@lib/core/controllers/page_state_service.dart` around lines 125 - 136, Update PageStateService’s clearSessionData and PageDataLoader response handling to use a session/request generation token: increment the token when clearing session data, capture the current generation when starting each load, and ignore responses whose generation is no longer current before writing to observables. Preserve the existing state reset and cache-clearing behavior.lib/core/controllers/page_data_loader.dart (1)
295-316: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftKeep the requested Likes segment attached to its asynchronous response.
The loader captures one segment before awaiting, while both response application and cache synchronization use mutable current state afterward.
lib/core/controllers/page_data_loader.dart#L295-L316: verify the requested segment/revision is still active before updating visible state.lib/core/controllers/page_state_service.dart#L751-L760: accept the requested segment explicitly rather than readingcurrentLikesSegment.🤖 Prompt for 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. In `@lib/core/controllers/page_data_loader.dart` around lines 295 - 316, Keep the captured Likes segment and request revision tied to the asynchronous response: in lib/core/controllers/page_data_loader.dart:295-316, verify they are still active before applying visible state or synchronizing the cache, and pass the captured segment to the cache-sync call. In lib/core/controllers/page_state_service.dart:751-760, update syncLikesSegmentCacheFromVisible to accept the requested segment explicitly and use it instead of reading mutable currentLikesSegment.
🧹 Nitpick comments (2)
android/gradle/wrapper/gradle-wrapper.properties (1)
5-5: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winPin the Gradle distribution checksum.
The wrapper now downloads
gradle-8.14-all.zipwithoutdistributionSha256Sum. Gradle recommends checksum verification; add the official checksumefe9a3d147d948d7528a9887fa35abcf24ca1a43ad06439996490f77569b02d1. (docs.gradle.org)Proposed fix
distributionUrl=https\://services.gradle.org/distributions/gradle-8.14-all.zip +distributionSha256Sum=efe9a3d147d948d7528a9887fa35abcf24ca1a43ad06439996490f77569b02d1🤖 Prompt for 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. In `@android/gradle/wrapper/gradle-wrapper.properties` at line 5, Add the official Gradle 8.14 distribution checksum as the distributionSha256Sum property alongside distributionUrl in the Gradle wrapper configuration, using the provided SHA-256 value.Source: MCP tools
lib/core/controllers/page_state_service.dart (1)
544-544: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the changed public mutation APIs and their cache side effects.
Add API documentation for
recordSwipe,isSessionSwiped,removePropertyFromLikes,addPropertyToLikes, andaddPropertyToPassed.As per coding guidelines, “Annotate public APIs and exports.”
Also applies to: 629-629, 672-702
🤖 Prompt for 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. In `@lib/core/controllers/page_state_service.dart` at line 544, Document the public mutation and query APIs recordSwipe, isSessionSwiped, removePropertyFromLikes, addPropertyToLikes, and addPropertyToPassed with Dart API comments, describing their behavior and relevant cache side effects. Keep the implementation unchanged and ensure each public method is annotated consistently with the project’s documentation guidelines.Source: Coding guidelines
🤖 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 `@android/settings.gradle.kts`:
- Line 22: Align the Kotlin Android plugin declaration in settings.gradle.kts
with a supported Android Gradle Plugin version: either downgrade AGP to 8.11.1
for Kotlin 2.2.20 or update Kotlin to a version officially supporting AGP
8.12.1. Keep the selected Kotlin and AGP versions as a compatible pair across
the Android build configuration.
In `@lib/core/controllers/page_data_loader.dart`:
- Around line 329-348: Update the refresh flow around _pageState.getStateForPage
and reinsertPropertyToDiscover so Discover responses cannot overwrite properties
restored locally while the request is pending. Track the post-request-start
local mutation/revision and merge or reject the stale response, preserving
reinserted cards while still applying valid backend results and session-swiped
filtering.
In `@lib/core/controllers/page_state_service.dart`:
- Around line 545-579: Update recordSwipe and its callers, including
LikesController.removeFromLikes() and moveToLikes(), to preserve and pass the
affected PropertyModel before removing it from the visible segment. Use that
model for optimistic destination-cache insertion and visible-list updates when
_findPropertyInAnyList() returns null, while retaining the existing
source-removal behavior.
- Around line 725-748: Update mergeLikesServerResults so each server item is
skipped when its ID exists in the selected segment’s opposite optimistic map,
preventing a stale result from reintroducing an opposite swipe. Only remove the
ID from optimistic when the server item belongs to the requested segment; do not
clear opposite or add skipped items to merged, while preserving pending
optimistic items and existing merge ordering.
In `@lib/core/services/google_places_service.dart`:
- Around line 218-228: Update the location-bias logic in the currentPosition
query-parameter block to apply location and capped radius whenever
currentPosition is available, removing the placesStrictBounds exclusion so
strict mode does not lose all bias. Preserve the existing radius parsing and
25–200 km clamping behavior.
In `@lib/features/discover/presentation/widgets/property_swipe_stack.dart`:
- Around line 41-46: Preserve an accessible non-drag path for Like and Pass in
the swipe stack described by the widget documentation. Add semantic custom
actions or equivalent accessible controls for both actions without restoring the
removed floating action bar, and connect them to the existing Like/Pass handling
used by the swipe gestures.
---
Outside diff comments:
In `@lib/core/controllers/page_data_loader.dart`:
- Around line 295-316: Keep the captured Likes segment and request revision tied
to the asynchronous response: in
lib/core/controllers/page_data_loader.dart:295-316, verify they are still active
before applying visible state or synchronizing the cache, and pass the captured
segment to the cache-sync call. In
lib/core/controllers/page_state_service.dart:751-760, update
syncLikesSegmentCacheFromVisible to accept the requested segment explicitly and
use it instead of reading mutable currentLikesSegment.
In `@lib/core/controllers/page_state_service.dart`:
- Around line 581-588: Update the swipe flow containing
_sessionSwipedPropertyIds, removePropertyFromDiscover, and
_swipesPort.recordSwipe to capture the affected local state and wrap the network
call in try/catch. On failure, restore Discover and cache/session state from the
captured snapshot, map the exception through error_mapper.dart, handle it with
error_handler.dart, and rethrow the mapped error so callers receive the failure.
- Around line 125-136: Update PageStateService’s clearSessionData and
PageDataLoader response handling to use a session/request generation token:
increment the token when clearing session data, capture the current generation
when starting each load, and ignore responses whose generation is no longer
current before writing to observables. Preserve the existing state reset and
cache-clearing behavior.
---
Nitpick comments:
In `@android/gradle/wrapper/gradle-wrapper.properties`:
- Line 5: Add the official Gradle 8.14 distribution checksum as the
distributionSha256Sum property alongside distributionUrl in the Gradle wrapper
configuration, using the provided SHA-256 value.
In `@lib/core/controllers/page_state_service.dart`:
- Line 544: Document the public mutation and query APIs recordSwipe,
isSessionSwiped, removePropertyFromLikes, addPropertyToLikes, and
addPropertyToPassed with Dart API comments, describing their behavior and
relevant cache side effects. Keep the implementation unchanged and ensure each
public method is annotated consistently with the project’s documentation
guidelines.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0602ecdc-48ba-4cc9-98a5-26725b9f43df
📒 Files selected for processing (31)
android/app/build.gradle.ktsandroid/app/src/main/AndroidManifest.xmlandroid/gradle.propertiesandroid/gradle/wrapper/gradle-wrapper.propertiesandroid/settings.gradle.ktslib/core/config/dev_env.g.dartlib/core/controllers/page_data_loader.dartlib/core/controllers/page_state_service.dartlib/core/data/models/popular_city.dartlib/core/services/google_places_service.dartlib/core/translations/app_translations.dartlib/core/widgets/common/location_selector.dartlib/features/discover/presentation/widgets/property_swipe_card.dartlib/features/discover/presentation/widgets/property_swipe_stack.dartlib/features/discover/presentation/widgets/swipe_card_action_buttons.dartlib/features/likes/presentation/controllers/likes_controller.dartlib/features/location_search/presentation/views/location_search_view.darttest/core/controllers/page_data_loader_test.darttest/core/controllers/page_filter_manager_test.darttest/core/controllers/page_state_service_test.darttest/core/data/models/popular_city_test.darttest/core/firebase/firebase_initializer_enabled_test.darttest/core/utils/error_handler_test.darttest/features/assistant/presentation/widgets/chat_message_bubble_test.darttest/features/auth/data/auth_repository_test.darttest/features/discover/presentation/views/discover_view_test.darttest/features/discover/presentation/widgets/property_swipe_stack_test.darttest/features/explore/presentation/views/explore_view_test.darttest/features/location_search/presentation/views/location_search_view_test.darttest/features/property_details/presentation/views/property_details_view_test.darttest/helpers/fake_webview_platform.dart
💤 Files with no reviewable changes (6)
- lib/features/discover/presentation/widgets/swipe_card_action_buttons.dart
- test/features/explore/presentation/views/explore_view_test.dart
- test/helpers/fake_webview_platform.dart
- lib/core/config/dev_env.g.dart
- test/core/firebase/firebase_initializer_enabled_test.dart
- lib/core/translations/app_translations.dart
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 31 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Resolve CodeRabbit and Cubic findings: pass PropertyModel into recordSwipe, preserve undo reinserts via discover mutation epoch, skip opposite optimistic likes on server merge, guard likes segment mid-flight switches, restore Places location bias semantics, extract popular suggestions helper, and add swipe-stack semantic actions for a11y. Align AGP to 8.11.1 with Kotlin 2.2.20.
Collaborator
Author
Addressed review comments (f4eeab8)Fixed CodeRabbit + Cubic findings:
Tests: page_state_service, page_data_loader, popular_city, likes_controller, location_search_view, property_swipe_stack — all passed. |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
lib/core/controllers/page_data_loader.dart (1)
209-223: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winReject responses when the request criteria changed in flight.
Both paths apply results fetched with the old location, filters, search query, or cursor to the latest state. A concurrent filter/location change can therefore mix unrelated properties and replace the new cursor with an obsolete one. Capture a request generation/signature and discard the response unless it still matches the latest state.
Also applies to: 331-373
🤖 Prompt for 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. In `@lib/core/controllers/page_data_loader.dart` around lines 209 - 223, Update the pagination response handling around _pageState.getStateForPage and the corresponding path near the second apply-results block to capture the request’s location, filters, search query, cursor, or generation before awaiting. After the response returns, compare that signature with the latest page state and discard the response when criteria changed; only merge properties and update nextCursor, hasMore, and isLoadingMore when it still matches.test/features/likes/presentation/controllers/likes_controller_test.dart (1)
45-51: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winStub the no-
propertyrecordSwipecall too.addToFavouritesandremoveFromFavouritesstill callrecordSwipewith onlypropertyIdandisLiked, so this stub may miss those invocations. Add a secondwhenfor the 2-arg shape.🤖 Prompt for 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. In `@test/features/likes/presentation/controllers/likes_controller_test.dart` around lines 45 - 51, Add a second stub alongside the existing mockPageStateService.recordSwipe setup for calls from addToFavourites and removeFromFavourites that provide only propertyId and isLiked, leaving the current three-argument stub unchanged.Source: Coding guidelines
🤖 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 `@lib/core/data/models/popular_city.dart`:
- Line 109: Update mergeWithRemote to stop adding remote suggestion keys to the
seen set, so distinct remote places sharing the same mainText are preserved.
Keep deduplication against the appropriate existing suggestions, and leave the
call site assigning mergeWithRemote(query, remote) unchanged.
---
Outside diff comments:
In `@lib/core/controllers/page_data_loader.dart`:
- Around line 209-223: Update the pagination response handling around
_pageState.getStateForPage and the corresponding path near the second
apply-results block to capture the request’s location, filters, search query,
cursor, or generation before awaiting. After the response returns, compare that
signature with the latest page state and discard the response when criteria
changed; only merge properties and update nextCursor, hasMore, and isLoadingMore
when it still matches.
In `@test/features/likes/presentation/controllers/likes_controller_test.dart`:
- Around line 45-51: Add a second stub alongside the existing
mockPageStateService.recordSwipe setup for calls from addToFavourites and
removeFromFavourites that provide only propertyId and isLiked, leaving the
current three-argument stub unchanged.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 332e2f0b-776e-45d9-8f21-861046fbec2b
📒 Files selected for processing (13)
android/settings.gradle.ktslib/core/controllers/page_data_loader.dartlib/core/controllers/page_state_service.dartlib/core/data/models/popular_city.dartlib/core/services/google_places_service.dartlib/core/widgets/common/location_selector.dartlib/features/discover/presentation/widgets/property_swipe_stack.dartlib/features/likes/presentation/controllers/likes_controller.dartlib/features/location_search/presentation/views/location_search_view.darttest/core/controllers/page_data_loader_test.darttest/core/controllers/page_state_service_test.darttest/core/data/models/popular_city_test.darttest/features/likes/presentation/controllers/likes_controller_test.dart
🚧 Files skipped from review as they are similar to previous changes (3)
- test/core/data/models/popular_city_test.dart
- lib/features/discover/presentation/widgets/property_swipe_stack.dart
- lib/core/controllers/page_state_service.dart
There was a problem hiding this comment.
1 issue found across 13 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="lib/core/controllers/page_state_service.dart">
<violation number="1" location="lib/core/controllers/page_state_service.dart:801">
P3: This update leaves the older likes cache sync helper unused and duplicates the fetch/cache responsibility in a second method. Keeping one likes-fetch merge path would make it easier to maintain and reduce the risk of the two helpers drifting apart.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Preserve only undo-reinserted Discover cards across fetches, queue likes reloads when segment switches mid-flight, and align likes load-more with optimistic merge. Soften Places radius clamp and document Gradle heap overrides; add regression tests.
Stop remote self-dedupe in popular city merge, animate a11y swipes from the drag threshold via a shared exit helper, use currentLikesSegment in the data loader, and drop the unused popularOnly list field.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@lib/core/controllers/page_data_loader.dart`:
- Around line 146-150: Update the page data loader’s disposal and queued-reload
flow to track disposed state, clear _pendingLikesReload during dispose(), and
prevent _finishActiveLoad from scheduling a reload after disposal. Also guard
the scheduled microtask before calling loadPageData so no repository request
starts once the loader is disposed.
- Around line 68-78: Introduce a per-page/query request generation in the page
data loading flow, incrementing it whenever filters, search, location, or cursor
change or a forced reload supersedes an active request. Capture the generation
when each request starts and validate it before any asynchronous completion
updates results, loading flags, error state, or appends load-more data; stale
completions must be discarded without mutating current state. Update the
active-load guard around _activeLoads and _pendingLikesReload so the newest
forced Explore/Discover/Likes reload is queued instead of dropped, including
while a Likes load-more request is active.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 71d927ce-2082-46a9-b81a-c896ad5e680d
📒 Files selected for processing (7)
android/gradle.propertieslib/core/controllers/page_data_loader.dartlib/core/controllers/page_state_service.dartlib/core/services/google_places_service.darttest/core/controllers/page_data_loader_test.darttest/core/controllers/page_state_service_test.darttest/core/data/models/popular_city_test.dart
🚧 Files skipped from review as they are similar to previous changes (4)
- test/core/data/models/popular_city_test.dart
- android/gradle.properties
- lib/core/services/google_places_service.dart
- lib/core/controllers/page_state_service.dart
Invalidate in-flight first-page and load-more completions with a per-page generation counter, queue force reloads for all tabs when superseded, and skip queued microtasks after dispose.
There was a problem hiding this comment.
All reported issues were addressed across 9 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
A failed load-more must not clear isLoadingMore after a force-refresh has superseded it; match the success-path generation check in catch.
saksham1991999
approved these changes
Jul 16, 2026
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Requires human review: Adjusts Android build operational parameters (JVM heap, workers, AGP/Kotlin versions) which requires operational context; UI change to gesture-only deck may need product sign-off.
Re-trigger cubic
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
dev_env.g.dartempty (no secrets)Review fixes applied before PR
dev_env.g.dartPopularCity.mergeWithRemoteto exact mainText de-dupe only (avoids dropping Greater Noida / similar areas)Test plan
flutter testfor popular_city, page_state_service, page_data_loader, location_search_view, property_swipe_stack, discover_view (133 passed)flutter analyzeclean (pre-commit)Summary by cubic
Stabilizes Discover/Likes so swiped cards don’t reappear and just‑liked items don’t flash. Improves location search with offline NCR cities and a softer Places bias; the deck is gesture‑only with accessible swipe actions, and page loads now drop stale responses via generation guards.
Bug Fixes
isLoadingMoreafter a superseding refresh); queued force‑reloads when superseded; dispose‑safe microtask skips.PropertyModeltorecordSwipe; queued a reload when segments switch mid‑flight using the current segment; aligned load‑more with optimistic state.New Features
popular:IDs locally; preferred non‑empty Google results with OSM fallback; restored a softer bias (no strict bounds, capped radius); stopped remote self‑de‑dupe in popular merge to avoid dropping similar areas.Written for commit 6b99446. Summary will update on new commits.
Summary by CodeRabbit