Feat/display user name in partition - #757
Conversation
📝 WalkthroughWalkthroughPartition membership responses now include user display names when available. Backend tests cover resolved and missing users, while frontend types and partition administration views display names with user ID fallbacks. ChangesPartition member display names
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install timed out. The project may have too many dependencies for the sandbox. 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: 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 `@openrag/services/orchestrators/partition_service.py`:
- Around line 677-680: Bound the concurrent user lookups in the partition
membership enrichment flow around _membership_repo.list_partition_members and
_user_repo.get_user. Fetch member users in bounded chunks, preserving the
existing member order and display_name assignment while avoiding one unbounded
asyncio.gather call for large partitions.
In `@ui/src/pages/admin/partitions/detail.tsx`:
- Around line 479-483: The member rows currently hide user_id when display_name
exists, making accounts ambiguous. Update the rendering near the user
display-name cells, including both affected occurrences, to always keep user_id
visible as secondary text, a tooltip, or in the removal confirmation while
preserving the existing display-name presentation.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 75463cd4-82e1-40b0-b2f3-e85f7a867969
📒 Files selected for processing (4)
openrag/services/orchestrators/partition_service.pytests/unit/services/orchestrators/test_partition_service.pyui/src/lib/api/partitions.tsui/src/pages/admin/partitions/detail.tsx
| members = await self._membership_repo.list_partition_members(partition) | ||
| users = await asyncio.gather(*(self._user_repo.get_user(m["user_id"]) for m in members)) | ||
| for member, user in zip(members, users, strict=True): | ||
| member["display_name"] = user.display_name if user else None |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
ast-grep outline openrag/services/orchestrators/partition_service.py --view expandedRepository: linagora/openrag
Length of output: 3527
🏁 Script executed:
sed -n '665,685p' openrag/services/orchestrators/partition_service.py
printf '\n---\n'
rg -n "def get_user|class .*User|get_user\\(" openrag -g '*.py'Repository: linagora/openrag
Length of output: 4635
🏁 Script executed:
sed -n '90,125p' openrag/services/persistence/user_repo.py
printf '\n---\n'
sed -n '1,120p' openrag/core/ports/user_repo.py
printf '\n---\n'
rg -n "get_users|list_users_by|batch.*user|user_ids" openrag/services openrag/core -g '*.py'Repository: linagora/openrag
Length of output: 3415
Bound the membership user lookups. asyncio.gather(...) launches one DB call per member, and UserRepository has no batch fetch here. Large partitions can overwhelm the pool; chunk these lookups or add a bulk user query.
🤖 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 `@openrag/services/orchestrators/partition_service.py` around lines 677 - 680,
Bound the concurrent user lookups in the partition membership enrichment flow
around _membership_repo.list_partition_members and _user_repo.get_user. Fetch
member users in bounded chunks, preserving the existing member order and
display_name assignment while avoiding one unbounded asyncio.gather call for
large partitions.
| <TableCell className="text-sm"> | ||
| {user.display_name || ( | ||
| <span className="font-mono text-muted-foreground">{user.user_id}</span> | ||
| )} | ||
| </TableCell> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Keep a stable identifier visible with display names.
Lines 480-482 and 513 show only the display name when available. Display names are not guaranteed to be unique, so administrators may be unable to distinguish members or confirm which account will be removed. Keep user_id as secondary text, a tooltip, or include it in the confirmation.
Also applies to: 511-514
🤖 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 `@ui/src/pages/admin/partitions/detail.tsx` around lines 479 - 483, The member
rows currently hide user_id when display_name exists, making accounts ambiguous.
Update the rendering near the user display-name cells, including both affected
occurrences, to always keep user_id visible as secondary text, a tooltip, or in
the removal confirmation while preserving the existing display-name
presentation.
3 participants
In the user table for a partition, display name instead of ID
Summary by CodeRabbit