Open
Conversation
…itory used in both home screen and route screen
…dable information
📝 WalkthroughWalkthroughThis PR centralizes search functionality by introducing a new Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant HomeVM as HomeViewModel
participant UnifiedSearch as UnifiedSearchRepository
participant RouteRepo as RouteRepository
participant EcosystemRepos as EateryRepository<br/>GymRepository<br/>LibraryRepository<br/>PrinterRepository
participant UI as HomeScreen UI
User->>HomeVM: Enter search query
HomeVM->>HomeVM: debounce query (homeQueryFlow)
HomeVM->>RouteRepo: makeSearch(query)
HomeVM->>UnifiedSearch: mergedSearchResults(queryFlow)
par Route Search Path
RouteRepo->>RouteRepo: Increment search token
RouteRepo->>RouteRepo: Validate token (prevent stale updates)
RouteRepo-->>UnifiedSearch: placeFlow (route results)
and Ecosystem Search Path
UnifiedSearch->>EcosystemRepos: Collect printers, gyms, eateries, libraries
EcosystemRepos-->>UnifiedSearch: Ecosystem data flows
UnifiedSearch->>UnifiedSearch: buildEcosystemSearchPlaces()
UnifiedSearch->>UnifiedSearch: mergeAndRankSearchResults()
end
UnifiedSearch->>UnifiedSearch: Combine & rank by relevance score
UnifiedSearch-->>HomeVM: addSearchResultsFlow (merged results)
HomeVM-->>UI: Update search results
UI->>UI: Render places with<br/>ecosystem icons & sublabels
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
Comment Tip You can disable poems in the walkthrough.Disable the |
RyanCheung555
commented
Mar 19, 2026
Comment on lines
+483
to
497
| //Hard-coded way to handle closed for construction message, change when backend is updated | ||
| val constructionAlert = "CLOSED FOR CONSTRUCTION" | ||
| val constructionRegex = Regex("""\bCLOSED\s+FOR\s+CONSTRUCTION\b""", RegexOption.IGNORE_CASE) | ||
| val hasConstructionAlert = constructionRegex.containsMatchIn(location) | ||
|
|
||
| val rawTitle = location.substringBefore("*").trim() | ||
| val title = rawTitle | ||
| .replace(constructionRegex, "") | ||
| .replace(Regex("""\s{2,}"""), " ") | ||
| .trim(' ', '-', ',', ';', ':') | ||
|
|
||
| val starAlertMessage = location.substringAfter("*", "").trim('*').trim() | ||
| val alertMessage = if (hasConstructionAlert) constructionAlert else starAlertMessage | ||
|
|
||
| return PrinterCardUiState( |
Contributor
Author
There was a problem hiding this comment.
Hard-coded implementation for now, waiting on backend pull request to be approved/merged in
RyanCheung555
commented
Mar 19, 2026
Comment on lines
+507
to
+517
| private fun String.toTitleCaseWords(): String { | ||
| return split(Regex("""\s+""")) | ||
| .filter { it.isNotBlank() } | ||
| .joinToString(" ") { word -> | ||
| word.lowercase(Locale.getDefault()) | ||
| .replaceFirstChar { char -> | ||
| if (char.isLowerCase()) char.titlecase(Locale.getDefault()) else char.toString() | ||
| } | ||
| } | ||
| } | ||
|
|
Contributor
Author
There was a problem hiding this comment.
Same as above ^^^
There was a problem hiding this comment.
Pull request overview
Adds a unified search pipeline so “ecosystem” places (eateries, gyms, printers, libraries) appear in the same search UX as backend route search results, with some related UI tweaks for search/favorites displays.
Changes:
- Introduces a merge + relevance-ranking layer for combining backend place search results with ecosystem places.
- Updates Home/Route view models and UI to consume merged search results and show ecosystem-specific icons/labels.
- Tweaks ecosystem bottom sheet rendering (distance formatting, gym open/capacity display, printer/library text cleanup, empty/error states).
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| app/src/main/java/com/cornellappdev/transit/util/ecosystem/PlaceSearchMerge.kt | New utilities to build ecosystem Place list and merge/rank results against backend search. |
| app/src/main/java/com/cornellappdev/transit/util/TransitConstants.kt | Adds METERS_TO_FEET constant for distance formatting. |
| app/src/main/java/com/cornellappdev/transit/ui/viewmodels/RouteViewModel.kt | Switches route search UI to use merged/unified search results flow. |
| app/src/main/java/com/cornellappdev/transit/ui/viewmodels/HomeViewModel.kt | Adds unified search usage for home + add-favorites search; adds distance/address/printer formatting helpers. |
| app/src/main/java/com/cornellappdev/transit/ui/screens/RouteScreen.kt | Clears query when route search sheets are dismissed; increases padding on search results. |
| app/src/main/java/com/cornellappdev/transit/ui/screens/HomeScreen.kt | Uses merged add-favorites search results; passes new formatting callbacks into bottom sheet. |
| app/src/main/java/com/cornellappdev/transit/ui/components/home/EcosystemBottomSheetContent.kt | Adds empty/error states; trims library addresses; printer card mapping; hides gym capacity when closed. |
| app/src/main/java/com/cornellappdev/transit/ui/components/SearchSuggestions.kt | Adds vertical padding between suggested items. |
| app/src/main/java/com/cornellappdev/transit/ui/components/MenuItem.kt | Adds place-type-based icon resolution and standardized ecosystem sublabels; supports caller-provided modifiers. |
| app/src/main/java/com/cornellappdev/transit/ui/components/LoadingLocationItems.kt | Adds list padding/spacing for search results layout. |
| app/src/main/java/com/cornellappdev/transit/models/search/UnifiedSearchRepository.kt | New repository to combine backend search flow with ecosystem places flow. |
| app/src/main/java/com/cornellappdev/transit/models/RouteRepository.kt | Adds a token guard so only the latest search request updates placeFlow. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+483
to
+486
| //Hard-coded way to handle closed for construction message, change when backend is updated | ||
| val constructionAlert = "CLOSED FOR CONSTRUCTION" | ||
| val constructionRegex = Regex("""\bCLOSED\s+FOR\s+CONSTRUCTION\b""", RegexOption.IGNORE_CASE) | ||
| val hasConstructionAlert = constructionRegex.containsMatchIn(location) |
| } | ||
| Image( | ||
| painterResource(iconForPlaceType(type)), | ||
| contentDescription = type.name, |
| const val NOTIFICATIONS_ENABLED = false No newline at end of file | ||
| const val NOTIFICATIONS_ENABLED = false | ||
|
|
||
| const val METERS_TO_FEET = 3.28 No newline at end of file |
Comment on lines
+51
to
+62
| fun mergedSearchResults(queryFlow: Flow<String>): Flow<ApiResponse<List<Place>>> = | ||
| combine( | ||
| queryFlow, | ||
| routeRepository.placeFlow, | ||
| ecosystemSearchPlacesFlow | ||
| ) { query, routeSearchResults, ecosystemPlaces -> | ||
| mergeAndRankSearchResults( | ||
| query = query, | ||
| routeSearchResults = routeSearchResults, | ||
| ecosystemPlaces = ecosystemPlaces | ||
| ) | ||
| } |
Comment on lines
+421
to
+432
| var distance: String | ||
| val distanceInMeters = calculateDistance( | ||
| LatLng( | ||
| currentLocationSnapshot.latitude, | ||
| currentLocationSnapshot.longitude | ||
| ), LatLng(latitude, longitude) | ||
| ).toString() | ||
| if (distanceInMeters.toDouble() > 160) { | ||
| distance = distanceInMeters.fromMetersToMiles() + " mi" | ||
| } else { | ||
| distance = (distanceInMeters.toDouble() * METERS_TO_FEET).toInt().toString() + " ft" | ||
| } |
| import kotlinx.coroutines.launch | ||
| import javax.inject.Inject | ||
| import java.util.Locale | ||
| import kotlin.text.toDouble |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
app/src/main/java/com/cornellappdev/transit/util/TransitConstants.kt (1)
17-17: Consider using a more precise conversion factor.The exact conversion is 3.28084 feet per meter. The current value of 3.28 introduces ~0.03% error, which is acceptable for display purposes but could be refined.
-const val METERS_TO_FEET = 3.28 +const val METERS_TO_FEET = 3.28084🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/cornellappdev/transit/util/TransitConstants.kt` at line 17, The constant METERS_TO_FEET uses an imprecise conversion (3.28); update the constant METERS_TO_FEET to the more precise conversion factor 3.28084 to reduce conversion error—locate the METERS_TO_FEET declaration in TransitConstants (const val METERS_TO_FEET) and replace the literal with 3.28084.app/src/main/java/com/cornellappdev/transit/util/ecosystem/PlaceSearchMerge.kt (1)
95-103: Potential deduplication issue with floating-point coordinates.Using
latitude.toString()andlongitude.toString()in the stable ID may cause the same logical place from different sources to not deduplicate if coordinates differ slightly (e.g.,42.4534vs42.45340001).Consider rounding coordinates or excluding them if
type,name, anddetailare sufficient for deduplication.💡 Suggested fix
private fun placeSearchStableId(place: Place): String { + // Round coordinates to reduce floating-point precision issues + val roundedLat = "%.5f".format(place.latitude) + val roundedLng = "%.5f".format(place.longitude) return listOf( place.type.name, place.name, place.detail.orEmpty(), - place.latitude.toString(), - place.longitude.toString() + roundedLat, + roundedLng ).joinToString("|") }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/cornellappdev/transit/util/ecosystem/PlaceSearchMerge.kt` around lines 95 - 103, The stable ID function placeSearchStableId currently uses raw latitude/longitude strings which can differ by tiny floating-point variations and prevent deduplication; update placeSearchStableId to normalize coordinates by rounding both place.latitude and place.longitude to a fixed precision (e.g., 6 decimal places) or format them with a consistent locale-specific formatter before joining, or if coordinates are unnecessary for your dedupe semantics, remove latitude/longitude from the join and rely on place.type.name, place.name, and place.detail; make this change inside the placeSearchStableId function so all callers get the normalized stable ID.app/src/main/java/com/cornellappdev/transit/models/RouteRepository.kt (1)
167-168: SwallowedCancellationExceptionis intentional but could use a brief comment.The static analysis tool flagged this as a swallowed exception. While ignoring
CancellationExceptionis the correct behavior here (we don't want cancelled coroutines to update the flow), adding a brief comment would improve clarity and silence future warnings.📝 Suggested improvement
- } catch (_: kotlinx.coroutines.CancellationException) { - // Ignore cancellation; latest query owns the flow update. + } catch (_: kotlinx.coroutines.CancellationException) { + // Intentionally ignored: cancelled coroutines should not update placeFlow. + // The latest (non-cancelled) query will handle the update. } catch (e: Exception) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/cornellappdev/transit/models/RouteRepository.kt` around lines 167 - 168, In RouteRepository, the catch block that swallows kotlinx.coroutines.CancellationException (the one guarding the flow update) should include a brief explanatory comment stating that swallowing is intentional so cancelled coroutines don't update the flow; update the existing comment to clearly mention that this is deliberate and that CancellationException is used for cooperative cancellation (or add a `@Suppress`("SwallowedException") annotation if your linter requires it) so static analysis warnings are silenced.app/src/main/java/com/cornellappdev/transit/ui/viewmodels/HomeViewModel.kt (1)
499-499: Preserve acronyms and room codes when title-casing printer subtitles.Lines 511-514 lowercase every token before capitalizing the first character, so already-correct values like
OIT,2B, orB/Wturn intoOit,2b, andB/w. Since this now feeds printer location text, it can regress some existing metadata.♻️ Safer casing approach
private fun String.toTitleCaseWords(): String { return split(Regex("""\s+""")) .filter { it.isNotBlank() } .joinToString(" ") { word -> - word.lowercase(Locale.getDefault()) - .replaceFirstChar { char -> - if (char.isLowerCase()) char.titlecase(Locale.getDefault()) else char.toString() - } + val letters = word.filter(Char::isLetter) + val preserveWord = + word.any(Char::isDigit) || + (letters.isNotEmpty() && letters.all(Char::isUpperCase)) + + if (preserveWord) { + word + } else { + word.lowercase(Locale.getDefault()) + .replaceFirstChar { char -> + if (char.isLowerCase()) char.titlecase(Locale.getDefault()) else char.toString() + } + } } }Also applies to: 507-516
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/cornellappdev/transit/ui/viewmodels/HomeViewModel.kt` at line 499, The current subtitle assignment uses description.substringAfter(...) and toTitleCaseWords() which lowercases every token before capitalizing, causing acronyms/room codes like OIT, 2B, B/W to be mangled; update the casing logic (either in the toTitleCaseWords() helper or inline where subtitle is assigned) to preserve tokens that are already all-uppercase or contain digits or non-letter characters: for each token, if token.matches(ALL_UPPERCASE) or token.containsDigitOrSymbol then keep it as-is, else capitalize only the first character and leave the rest lowercase; apply the same change to the other related transformations around the subtitle handling to avoid regressing printer location metadata.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/main/java/com/cornellappdev/transit/models/RouteRepository.kt`:
- Around line 148-175: The current makeSearch function sets _placeFlow.value =
ApiResponse.Pending before launching the coroutine, creating a race where a
newer search's result can be overwritten; change the code to assign
ApiResponse.Pending inside the launched coroutine only after verifying the local
token still equals latestSearchToken.get() (use the token variable), and keep
all subsequent state updates (_placeFlow.value =
ApiResponse.Success/ApiResponse.Error) guarded by the same token check so stale
coroutines cannot overwrite newer results.
In
`@app/src/main/java/com/cornellappdev/transit/models/search/UnifiedSearchRepository.kt`:
- Around line 51-62: The combine in mergedSearchResults currently pairs
queryFlow with a shared routeRepository.placeFlow, which lets prior route
results bleed into new queries; change the merge so route results are bound to
the originating query (either by carrying the query/token inside placeFlow
upstream or by making mergedSearchResults use flatMapLatest on queryFlow to
subscribe to a query-scoped route search flow) so that mergeAndRankSearchResults
always receives route results tied to the same query and prevents stale route
data from masking pending/error states (see routeRepository.placeFlow and
mergeAndRankSearchResults and PlaceSearchMerge.kt for where backend misses are
downgraded).
In `@app/src/main/java/com/cornellappdev/transit/ui/viewmodels/HomeViewModel.kt`:
- Around line 421-433: The distance unit cutoff uses a hardcoded 160 meters;
replace that with an exact 0.1-mile threshold converted to meters (0.1 *
1609.344 = 160.9344) and compare the numeric distance instead of the string:
compute distanceInMeters as a Double from calculateDistance(...) and use if
(distanceInMeters >= 0.1 * 1609.344) { ... } else { ... } (reference symbols:
calculateDistance, distanceInMeters, METERS_TO_FEET).
---
Nitpick comments:
In `@app/src/main/java/com/cornellappdev/transit/models/RouteRepository.kt`:
- Around line 167-168: In RouteRepository, the catch block that swallows
kotlinx.coroutines.CancellationException (the one guarding the flow update)
should include a brief explanatory comment stating that swallowing is
intentional so cancelled coroutines don't update the flow; update the existing
comment to clearly mention that this is deliberate and that
CancellationException is used for cooperative cancellation (or add a
`@Suppress`("SwallowedException") annotation if your linter requires it) so static
analysis warnings are silenced.
In `@app/src/main/java/com/cornellappdev/transit/ui/viewmodels/HomeViewModel.kt`:
- Line 499: The current subtitle assignment uses description.substringAfter(...)
and toTitleCaseWords() which lowercases every token before capitalizing, causing
acronyms/room codes like OIT, 2B, B/W to be mangled; update the casing logic
(either in the toTitleCaseWords() helper or inline where subtitle is assigned)
to preserve tokens that are already all-uppercase or contain digits or
non-letter characters: for each token, if token.matches(ALL_UPPERCASE) or
token.containsDigitOrSymbol then keep it as-is, else capitalize only the first
character and leave the rest lowercase; apply the same change to the other
related transformations around the subtitle handling to avoid regressing printer
location metadata.
In
`@app/src/main/java/com/cornellappdev/transit/util/ecosystem/PlaceSearchMerge.kt`:
- Around line 95-103: The stable ID function placeSearchStableId currently uses
raw latitude/longitude strings which can differ by tiny floating-point
variations and prevent deduplication; update placeSearchStableId to normalize
coordinates by rounding both place.latitude and place.longitude to a fixed
precision (e.g., 6 decimal places) or format them with a consistent
locale-specific formatter before joining, or if coordinates are unnecessary for
your dedupe semantics, remove latitude/longitude from the join and rely on
place.type.name, place.name, and place.detail; make this change inside the
placeSearchStableId function so all callers get the normalized stable ID.
In `@app/src/main/java/com/cornellappdev/transit/util/TransitConstants.kt`:
- Line 17: The constant METERS_TO_FEET uses an imprecise conversion (3.28);
update the constant METERS_TO_FEET to the more precise conversion factor 3.28084
to reduce conversion error—locate the METERS_TO_FEET declaration in
TransitConstants (const val METERS_TO_FEET) and replace the literal with
3.28084.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: bec13dbc-43ee-4cbf-8342-21925107468e
📒 Files selected for processing (12)
app/src/main/java/com/cornellappdev/transit/models/RouteRepository.ktapp/src/main/java/com/cornellappdev/transit/models/search/UnifiedSearchRepository.ktapp/src/main/java/com/cornellappdev/transit/ui/components/LoadingLocationItems.ktapp/src/main/java/com/cornellappdev/transit/ui/components/MenuItem.ktapp/src/main/java/com/cornellappdev/transit/ui/components/SearchSuggestions.ktapp/src/main/java/com/cornellappdev/transit/ui/components/home/EcosystemBottomSheetContent.ktapp/src/main/java/com/cornellappdev/transit/ui/screens/HomeScreen.ktapp/src/main/java/com/cornellappdev/transit/ui/screens/RouteScreen.ktapp/src/main/java/com/cornellappdev/transit/ui/viewmodels/HomeViewModel.ktapp/src/main/java/com/cornellappdev/transit/ui/viewmodels/RouteViewModel.ktapp/src/main/java/com/cornellappdev/transit/util/TransitConstants.ktapp/src/main/java/com/cornellappdev/transit/util/ecosystem/PlaceSearchMerge.kt
Comment on lines
148
to
175
| fun makeSearch(query: String) { | ||
| val token = latestSearchToken.incrementAndGet() | ||
|
|
||
| if (query.isBlank()) { | ||
| if (token == latestSearchToken.get()) { | ||
| _placeFlow.value = ApiResponse.Success(emptyList()) | ||
| } | ||
| return | ||
| } | ||
|
|
||
| _placeFlow.value = ApiResponse.Pending | ||
| CoroutineScope(Dispatchers.IO).launch { | ||
| try { | ||
| val placeResponse = appleSearch(SearchQuery(query)) | ||
| val res = placeResponse.unwrap() | ||
| val totalLocations = (res.places ?: emptyList()) + (res.stops ?: (emptyList())) | ||
| _placeFlow.value = ApiResponse.Success(totalLocations) | ||
| if (token == latestSearchToken.get()) { | ||
| _placeFlow.value = ApiResponse.Success(totalLocations) | ||
| } | ||
| } catch (_: kotlinx.coroutines.CancellationException) { | ||
| // Ignore cancellation; latest query owns the flow update. | ||
| } catch (e: Exception) { | ||
| _placeFlow.value = ApiResponse.Error | ||
| if (token == latestSearchToken.get()) { | ||
| _placeFlow.value = ApiResponse.Error | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Race condition: Pending state set before token validation.
Line 158 sets _placeFlow.value = ApiResponse.Pending unconditionally before launching the coroutine. If a newer search completes before this coroutine starts, the Pending state could briefly overwrite a valid Success result.
Consider moving the Pending assignment inside the coroutine, guarded by the token check:
🐛 Proposed fix
fun makeSearch(query: String) {
val token = latestSearchToken.incrementAndGet()
if (query.isBlank()) {
if (token == latestSearchToken.get()) {
_placeFlow.value = ApiResponse.Success(emptyList())
}
return
}
- _placeFlow.value = ApiResponse.Pending
CoroutineScope(Dispatchers.IO).launch {
try {
+ if (token == latestSearchToken.get()) {
+ _placeFlow.value = ApiResponse.Pending
+ }
val placeResponse = appleSearch(SearchQuery(query))
val res = placeResponse.unwrap()
val totalLocations = (res.places ?: emptyList()) + (res.stops ?: (emptyList()))
if (token == latestSearchToken.get()) {
_placeFlow.value = ApiResponse.Success(totalLocations)
}🧰 Tools
🪛 detekt (1.23.8)
[warning] 169-169: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/main/java/com/cornellappdev/transit/models/RouteRepository.kt` around
lines 148 - 175, The current makeSearch function sets _placeFlow.value =
ApiResponse.Pending before launching the coroutine, creating a race where a
newer search's result can be overwritten; change the code to assign
ApiResponse.Pending inside the launched coroutine only after verifying the local
token still equals latestSearchToken.get() (use the token variable), and keep
all subsequent state updates (_placeFlow.value =
ApiResponse.Success/ApiResponse.Error) guarded by the same token check so stale
coroutines cannot overwrite newer results.
Comment on lines
+51
to
+62
| fun mergedSearchResults(queryFlow: Flow<String>): Flow<ApiResponse<List<Place>>> = | ||
| combine( | ||
| queryFlow, | ||
| routeRepository.placeFlow, | ||
| ecosystemSearchPlacesFlow | ||
| ) { query, routeSearchResults, ecosystemPlaces -> | ||
| mergeAndRankSearchResults( | ||
| query = query, | ||
| routeSearchResults = routeSearchResults, | ||
| ecosystemPlaces = ecosystemPlaces | ||
| ) | ||
| } |
There was a problem hiding this comment.
Bind route results to the query that produced them.
Line 54 combines the active query with a shared routeRepository.placeFlow, not a query-scoped result stream. Cross-file, app/src/main/java/com/cornellappdev/transit/util/ecosystem/PlaceSearchMerge.kt downgrades backend misses to fallback scores instead of filtering them, so the previous route response can bleed into the next query and mask pending/error states until the fresh search lands. Carry the query/token with placeFlow, or make this merge flatMapLatest over a query-bound route search flow.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@app/src/main/java/com/cornellappdev/transit/models/search/UnifiedSearchRepository.kt`
around lines 51 - 62, The combine in mergedSearchResults currently pairs
queryFlow with a shared routeRepository.placeFlow, which lets prior route
results bleed into new queries; change the merge so route results are bound to
the originating query (either by carrying the query/token inside placeFlow
upstream or by making mergedSearchResults use flatMapLatest on queryFlow to
subscribe to a query-scoped route search flow) so that mergeAndRankSearchResults
always receives route results tied to the same query and prevents stale route
data from masking pending/error states (see routeRepository.placeFlow and
mergeAndRankSearchResults and PlaceSearchMerge.kt for where backend misses are
downgraded).
Comment on lines
+421
to
+433
| var distance: String | ||
| val distanceInMeters = calculateDistance( | ||
| LatLng( | ||
| currentLocationSnapshot.latitude, | ||
| currentLocationSnapshot.longitude | ||
| ), LatLng(latitude, longitude) | ||
| ).toString() | ||
| if (distanceInMeters.toDouble() > 160) { | ||
| distance = distanceInMeters.fromMetersToMiles() + " mi" | ||
| } else { | ||
| distance = (distanceInMeters.toDouble() * METERS_TO_FEET).toInt().toString() + " ft" | ||
| } | ||
| return " - $distance" |
There was a problem hiding this comment.
Use an exact 0.1-mile cutoff for the unit switch.
Line 428 uses 160 meters as the breakpoint, but 0.1 miles is about 160.934 meters / 528 feet. Distances between those values are still under 0.1 miles, so this flips to miles a bit too early.
💡 Suggested fix
- var distance: String
- val distanceInMeters = calculateDistance(
+ var distance: String
+ val distanceInMeters = calculateDistance(
LatLng(
currentLocationSnapshot.latitude,
currentLocationSnapshot.longitude
- ), LatLng(latitude, longitude)
- ).toString()
- if (distanceInMeters.toDouble() > 160) {
- distance = distanceInMeters.fromMetersToMiles() + " mi"
+ ),
+ LatLng(latitude, longitude)
+ )
+ val distanceInFeet = distanceInMeters * METERS_TO_FEET
+ if (distanceInFeet >= 528) {
+ distance = distanceInMeters.toString().fromMetersToMiles() + " mi"
} else {
- distance = (distanceInMeters.toDouble() * METERS_TO_FEET).toInt().toString() + " ft"
+ distance = distanceInFeet.toInt().toString() + " ft"
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| var distance: String | |
| val distanceInMeters = calculateDistance( | |
| LatLng( | |
| currentLocationSnapshot.latitude, | |
| currentLocationSnapshot.longitude | |
| ), LatLng(latitude, longitude) | |
| ).toString() | |
| if (distanceInMeters.toDouble() > 160) { | |
| distance = distanceInMeters.fromMetersToMiles() + " mi" | |
| } else { | |
| distance = (distanceInMeters.toDouble() * METERS_TO_FEET).toInt().toString() + " ft" | |
| } | |
| return " - $distance" | |
| var distance: String | |
| val distanceInMeters = calculateDistance( | |
| LatLng( | |
| currentLocationSnapshot.latitude, | |
| currentLocationSnapshot.longitude | |
| ), | |
| LatLng(latitude, longitude) | |
| ) | |
| val distanceInFeet = distanceInMeters * METERS_TO_FEET | |
| if (distanceInFeet >= 528) { | |
| distance = distanceInMeters.toString().fromMetersToMiles() + " mi" | |
| } else { | |
| distance = distanceInFeet.toInt().toString() + " ft" | |
| } | |
| return " - $distance" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/main/java/com/cornellappdev/transit/ui/viewmodels/HomeViewModel.kt`
around lines 421 - 433, The distance unit cutoff uses a hardcoded 160 meters;
replace that with an exact 0.1-mile threshold converted to meters (0.1 *
1609.344 = 160.9344) and compare the numeric distance instead of the string:
compute distanceInMeters as a Double from calculateDistance(...) and use if
(distanceInMeters >= 0.1 * 1609.344) { ... } else { ... } (reference symbols:
calculateDistance, distanceInMeters, METERS_TO_FEET).
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.
Overview
Changes Made
Test Coverage
Screenshots (delete if not applicable)
Ecosystem Search Example
ecosystem_search_recording.webm
Summary by CodeRabbit
New Features
Bug Fixes
UI/UX Improvements