Use short names for grouped projects#4138
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 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 |
github-actions
Bot
added
vouch:unvouched
b3d6acf to
923bc2f
Compare
github-actions
Bot
added
size:XS
923bc2f to
93f2851
Compare
github-actions
Bot
added
size:S
ApprovabilityVerdict: Needs human review Multiple unresolved review comments identify that this change can cause ambiguous project labels when different repositories share the same short name (e.g., both org/app and user/app showing as 'app'). These substantive concerns warrant human review. You can customize Macroscope's approvability policy. Learn more. |
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 93f285159e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| input.members.map((member) => member.repositoryIdentity?.name), | ||
| ); | ||
| if (sharedRepositoryNames.length === 1) { | ||
| return sharedRepositoryNames[0]!; |
There was a problem hiding this comment.
When two distinct canonical repositories with the same basename are each grouped across environments (for example, org/app and user/app), this returns app for both groups. Both the web sidebar and mobile home list call deriveProjectGroupLabel independently per group, so users can no longer distinguish the rows or their new-thread targets; the previous qualified displayName kept them distinct. Prefer the short name only when it does not collide with another visible repository group, or retain a qualified fallback for collisions.
Useful? React with 👍 / 👎.
93f2851 to
6435959
Compare
Dismissing prior approval to re-evaluate 6435959
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
6435959 to
0d49c21
Compare
| (project) => | ||
| project.title.toLocaleLowerCase().includes(query) || | ||
| project.repositoryIdentity?.displayName?.toLocaleLowerCase().includes(query), | ||
| ); |
There was a problem hiding this comment.
Search skips repository short name
Low Severity
Home search was extended to match repositoryIdentity.displayName, but not repositoryIdentity.name. When a name conflict yields a workspace title label instead of a short or qualified name, queries matching only the repo short name may not match the group even though threads exist.
Reviewed by Cursor Bugbot for commit 0d49c21. Configure here.
| const query = input.searchQuery.trim().toLocaleLowerCase(); | ||
| const visibleProjects = [...groups.values()] | ||
| .filter((group) => group.threads.length > 0 || group.pendingTasks.length > 0) | ||
| .flatMap((group) => group.projects); |
There was a problem hiding this comment.
Inactive repos skip collision scan
Medium Severity
Mobile builds visibleProjects only from home groups that already have threads or pending tasks. Another loaded repository with the same short name but no home activity is omitted, so deriveProjectGroupLabel treats the name as unique and shows an ambiguous short label while web includes that repository in its collision scan.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 0d49c21. Configure here.
0d49c21 to
e330656
Compare
e330656 to
9082dfb
Compare
9082dfb to
a9bbbc4
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
There are 3 total unresolved issues (including 2 from previous reviews).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit a9bbbc4. Configure here.
| if (sharedDisplayNames.length === 1) { | ||
| return sharedDisplayNames[0]!; | ||
| } | ||
|
|
There was a problem hiding this comment.
Ambiguous short labels across groups
Medium Severity
The deriveProjectGroupLabel function now uses the short repository name for a group's label if all its members share it. This check is limited to the group's members and doesn't consider other visible project groups. As a result, distinct project groups (e.g., org/app and user/app) can both be labeled app in the sidebar and home thread list, leading to ambiguity where qualified labels are expected.
Reviewed by Cursor Bugbot for commit a9bbbc4. Configure here.
1 participant
What Changed
Why
Repository grouping correctly uses a canonical remote identity, but the group label preferred
displayName, which can include a repository owner. The repository identity already exposes the shortname, so checking it first keeps cross-device grouping intact without showing the owner.UI Changes
Before
After
Checks
vp test apps/web/src/environmentGrouping.test.ts apps/mobile/src/features/home/homeThreadList.test.ts(18 passed)vp checkvp run typecheckChecklist