feat: hide top-window titles vis on android #737
Conversation
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed everything up to 1c08147 in 1 minute and 26 seconds. Click for details.
- Reviewed
13lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. src/components/SelectableVisualization.vue:31
- Draft comment:
Check that 'activityStore.android.available' is always defined and reactive. Consider moving the Android check into a computed property for consistency. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 15% vs. threshold = 50% This comment appears to be speculative and not actionable. The first part ("Check that...is always defined and reactive") is asking the author to verify something rather than pointing out a concrete issue. The second part ("Consider moving...into a computed property") is a refactoring suggestion but doesn't show strong evidence that there's an actual problem. Looking at the code,activityStore.android.availableis used elsewhere in the file (line 176) in the same way, so it's already an established pattern. The comment doesn't identify a specific bug or issue - it's more of a "make sure this works" type of comment, which violates the rules about not asking authors to confirm or verify things. Could there be a legitimate concern about reactivity or the property being undefined? Perhaps in some edge cases the android property might not exist on the store? The suggestion about computed properties could improve code organization and testability. Even if there were edge cases, the comment doesn't provide evidence that this is actually a problem. The pattern is already used elsewhere in the file (line 176), so if it were broken, it would be broken there too. The computed property suggestion is a refactoring idea but not a clear code quality issue that needs to be fixed. Without strong evidence of an actual problem, this falls into the category of speculative comments that should be removed. This comment should be deleted. It asks the author to verify/check something rather than pointing out a concrete issue, and the refactoring suggestion lacks strong justification. The pattern used is consistent with the rest of the file.
Workflow ID: wflow_lUqeCNYxa80H80uF
You can customize 
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #737 +/- ##
==========================================
+ Coverage 25.98% 26.30% +0.31%
==========================================
Files 27 29 +2
Lines 1643 1684 +41
Branches 279 288 +9
==========================================
+ Hits 427 443 +16
- Misses 1190 1219 +29
+ Partials 26 22 -4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Greptile OverviewGreptile SummaryThis PR attempts to hide the 'Top Window Titles' visualization on Android devices by adding a condition What ChangedA single line modification adds an Android availability check to prevent rendering the Critical Issues Found1. Breaks Multi-Device ScenariosThe implementation has a critical flaw for users running both Android and desktop watchers simultaneously. When both 2. Inconsistent Availability FlagThe Root Cause AnalysisThe underlying issue is that Android's Recommended FixRather than hiding at the rendering level, the fix should:
The default views already correctly exclude Confidence Score: 2/5
Important Files ChangedFile Analysis
Sequence DiagramsequenceDiagram
participant User
participant SelectableVisualization
participant ActivityStore
participant Queries
participant Server
Note over User,Server: Scenario: User with both Android and Desktop watchers
User->>ActivityStore: Load activity data
ActivityStore->>Server: get_buckets()
Server-->>ActivityStore: buckets (android + window)
ActivityStore->>ActivityStore: android.available = true<br/>window.available = true
alt Desktop Query Path
ActivityStore->>Queries: query_desktop_full()
Queries->>Server: fullDesktopQuery
Server-->>Queries: {app_events, title_events (window titles)}
Queries->>ActivityStore: query_window_completed(data)
ActivityStore->>ActivityStore: window.top_apps = app_events<br/>window.top_titles = title_events
end
alt Android Query Path (also runs)
ActivityStore->>Queries: query_android()
Queries->>Server: appQuery (merge by app+classname)
Server-->>Queries: {app_events, title_events (classnames)}
Queries->>ActivityStore: query_window_completed(data)
Note over ActivityStore: OVERWRITES window.top_titles<br/>with Android classnames!
end
User->>SelectableVisualization: View top_titles visualization
SelectableVisualization->>SelectableVisualization: Check: type == 'top_titles' &&<br/>!android.available
Note over SelectableVisualization: Evaluates to: true && !true = FALSE
SelectableVisualization->>User: ❌ Nothing rendered<br/>(Desktop titles hidden!)
Note over User,Server: ISSUE: Desktop window titles are lost<br/>even though they should be available
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Additional Comments (1)
Currently:
This creates confusing behavior:
For consistency, the This would make the availability check consistent with the rendering behavior and properly show "(no data)" in the dropdown when Android is present. Prompt To Fix With AIThis is a comment left during a code review.
Path: src/components/SelectableVisualization.vue
Line: 178:181
Comment:
The `available` flag for `top_titles` is inconsistent with the rendering condition on line 31.
Currently:
- Line 180: `available: this.activityStore.window.available` (doesn't account for Android)
- Line 31: Renders only if `type == 'top_titles' && !activityStore.android.available`
This creates confusing behavior:
1. On Android-only devices: `available = false` correctly shows "(no data)" in the dropdown
2. On devices with both Android and desktop: `available = true` (shows as available in dropdown), but line 31 prevents rendering (shows nothing)
For consistency, the `available` flag should match the actual rendering condition. If Android should prevent showing top_titles, then update this to:
```suggestion
top_titles: {
title: 'Top Window Titles',
available: this.activityStore.window.available && !this.activityStore.android.available,
},
```
This would make the availability check consistent with the rendering behavior and properly show "(no data)" in the dropdown when Android is present.
How can I resolve this? If you propose a fix, please make it concise. |
1 participant
Important
Hide 'top_titles' visualization on Android in
SelectableVisualization.vue.SelectableVisualization.vue, hide 'top_titles' visualization on Android by adding&& !activityStore.android.availablecondition to thev-ifdirective.This description was created by
for 1c08147. You can customize this summary. It will automatically update as commits are pushed.