Conversation
Collaborator
Author
This stack of pull requests is managed by Graphite. Learn more about stacking. |
This was referenced Apr 16, 2026
|
|
Contributor
📝 WalkthroughWalkthroughVirtual key usage now derives from assigned users' access profiles when present. A new hook fetches assigned users and profiles and exposes display budgets, rate-limits, and management flags; UI components use the hook, number formatting was centralized, and edit/delete flows are restricted for profile-managed virtual keys. Changes
sequenceDiagram
actor User
participant VKUI as VirtualKey Sheet / Table
participant Hook as useVirtualKeyUsage
participant API as RTK Query (virtualKeyUsers / accessProfiles)
participant UI as RateLimitDisplay / UsageLine
User->>VKUI: open view or table row
VKUI->>Hook: call useVirtualKeyUsage(vk)
Hook->>API: useGetVirtualKeyUsersQuery(vk.id)
API-->>Hook: assignedUsers[]
alt assignedUsers non-empty
Hook->>API: useGetUserAccessProfilesQuery(userId)
API-->>Hook: accessProfiles[]
Hook->>Hook: select managingProfile by virtual_key_ids
Hook-->>VKUI: displayBudgets, displayRateLimit, isManagedByProfile, assignedUsers
else no assignedUsers
Hook-->>VKUI: fallback to vk.budgets, vk.rate_limit
end
VKUI->>UI: render UsageLine / RateLimitDisplay using display data
UI-->>User: show progress bars, limits, and managed indicators
🎯 4 (Complex) | ⏱️ ~45 minutes
🚥 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)
Comment |
Contributor
Confidence Score: 5/5Safe to merge; remaining findings are P2 style/consistency items that do not block correct behavior. All previously flagged P1 concerns are addressed. The two remaining findings (CSV export inconsistency, isExhausted duplication) are P2 clean-up items with no runtime impact on the primary AP-backed usage display feature. virtualKeysTable.tsx (CSV export for managed VKs), virtualKeyDetailsSheet.tsx (isExhausted duplication) Important Files Changed
Reviews (6): Last reviewed commit: "feat: show live access-profile-backed us..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ui/app/workspace/virtual-keys/hooks/useVirtualKeyUsage.ts`:
- Around line 23-39: The current logic infers AP-management from
assignedUsers[0] which can misattribute profiles; instead, stop assuming the
first assigned user is the manager: fetch access profiles for all assigned users
(e.g., call useGetUserAccessProfilesQuery per assignedUsers[*] or a batched
endpoint), then compute managingProfile by searching those users' profiles for
one where profile.virtual_key_ids includes vk.id; set isManagedByProfile true
only if such a matching profile exists and otherwise leave managingProfile
undefined (so keys assigned directly or with multiple users are not incorrectly
marked AP-managed). Use the existing symbols useGetVirtualKeyUsersQuery,
vkUsersData / assignedUsers, useGetUserAccessProfilesQuery and managingProfile
to locate and replace the current managingUserId-based logic.
- Around line 41-67: The code currently falls back to vk.budgets and
vk.rate_limit when managingProfile exists but has empty budget_lines or no
rate_limits; update the logic so that presence of managingProfile (not presence
of items) determines the source: for displayBudgets, if managingProfile is
defined return managingProfile.budget_lines (map to Budget objects if present,
else return an empty array or undefined consistent with downstream expectations)
instead of falling back to vk.budgets; for displayRateLimit, if managingProfile
is defined then use apRL if present (construct RateLimit when hasApRateLimit) or
set displayRateLimit to undefined (do not fall back to vk.rate_limit); change
the ternary conditions around displayBudgets and displayRateLimit to check
managingProfile !== undefined rather than relying on .length or truthiness so
profile-managed keys never show raw vk limits.
In `@ui/app/workspace/virtual-keys/views/virtualKeySheet.tsx`:
- Around line 162-165: The code incorrectly sets isManagedByProfile using
assignedUsers length; change it to detect an actual access-profile association
on the virtual key instead of checking for assignees. Replace the
isManagedByProfile assignment (currently using useGetVirtualKeyUsersQuery,
assignedUsers and assignedUsers.length) with a check for the virtualKey's
access-profile identifier/property (e.g. virtualKey?.accessProfileId or
virtualKey?.managedByAccessProfile) or, if there is a dedicated hook (e.g.
useGetVirtualKeyAccessProfileQuery), use that to determine profile ownership so
the restricted submit path only activates when the key is truly owned by an
access profile.
In `@ui/app/workspace/virtual-keys/views/virtualKeysTable.tsx`:
- Around line 173-188: Add stable data-testid attributes to the dialog's confirm
and cancel controls so tests can target them: update the AlertDialogCancel
component to include data-testid="virtual-key-delete-cancel" and the
AlertDialogAction (the delete button that calls onDelete and uses isDeleting) to
include data-testid="virtual-key-delete-confirm"; ensure you add these to the
existing AlertDialogCancel and AlertDialogAction elements surrounding the
onDelete(vk.id) usage so selectors follow the pattern entity-element-qualifier.
- Around line 637-640: The row badge is still computing isExhausted from
vk.budgets / vk.rate_limit while VKBudgetCell and VKRateLimitCell now use the
AP-backed useVirtualKeyUsage hook; change the badge to derive exhaustion from
the same source by calling useVirtualKeyUsage (or consuming its returned
isExhausted/isExhaustedByBudget/isExhaustedByRateLimit) for the given vk (e.g.,
via vk.id) and use that value for the row-level isExhausted determination
instead of reading vk.budgets or vk.rate_limit directly so managed keys display
consistent Active/Exhausted state.
In `@ui/lib/utils/governance.ts`:
- Around line 34-39: The formatCompactNumber function currently produces
rollover artifacts (e.g., 999_950 -> "1000.0K" instead of "1.0M"); replace the
simple if-chain with a unit-scaling loop that divides the numeric value by 1000
until it's below 1000 or you hit the largest unit, then format with the desired
decimal places, but after formatting check if the rounded value equals 1000
(e.g., Number(formattedValue) === 1000); if so, divide once more and advance the
unit (promote K->M, M->B, etc.) before returning the final string—apply this
logic inside formatCompactNumber so boundary values are promoted to the next
unit correctly.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 60ad4dce-8b8b-49fd-a40a-7ce921713ca9
📒 Files selected for processing (7)
ui/app/workspace/dashboard/components/modelRankingsTab.tsxui/app/workspace/virtual-keys/hooks/useVirtualKeyUsage.tsui/app/workspace/virtual-keys/views/virtualKeyDetailsSheet.tsxui/app/workspace/virtual-keys/views/virtualKeySheet.tsxui/app/workspace/virtual-keys/views/virtualKeysTable.tsxui/components/rateLimitDisplay.tsxui/lib/utils/governance.ts
danpiths
force-pushed
the
feature/04-16-feat_consolidate_user_identity_context_key_and_propagate_user_name
branch
from
50ff0e0 to
af73b47
Compare
danpiths
force-pushed
the
feature/04-16-feat_show_live_access-profile-backed_usage_in_virtual_key_views
branch
from
0c02b29 to
3913095
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ui/app/workspace/virtual-keys/views/virtualKeyDetailsSheet.tsx`:
- Around line 92-99: The label "Assigned To" used for the assignedUsers list is
ambiguous with the other "Assigned To" row; change the Label text in the block
that renders assignedUsers (the JSX using assignedUsers.map(...) and the Label
component) to "Assigned Users" (or another distinct string like "Users
Assigned") so the two sections are clearly differentiated; update only the Label
content in the virtualKeyDetailsSheet component where assignedUsers is rendered.
In `@ui/app/workspace/virtual-keys/views/virtualKeySheet.tsx`:
- Around line 162-165: The hook useVirtualKeyUsage currently checks only
assignedUsers[0] to determine AP ownership, so update its implementation to
search across all assignedUsers for an access-profile that contains the virtual
key (e.g., iterate assignedUsers and for each fetch/check their
accessProfiles.virtual_key_ids includes virtualKey.id), return the first
matching managing profile and set isManagedByProfileHook true when any assigned
user’s profile manages the key; ensure the virtualKeySheet code continues to use
isManagedByProfileHook (and the virtualKey id) so editing is disabled when any
assignee’s profile claims ownership.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d789b33a-9d68-42b6-a5c3-7946e18abc16
📒 Files selected for processing (11)
ui/app/_fallbacks/enterprise/lib/store/apis/accessProfileApi.tsui/app/_fallbacks/enterprise/lib/store/apis/virtualKeyUsersApi.tsui/app/_fallbacks/enterprise/lib/types/accessProfile.tsui/app/_fallbacks/enterprise/lib/types/user.tsui/app/workspace/dashboard/components/modelRankingsTab.tsxui/app/workspace/virtual-keys/hooks/useVirtualKeyUsage.tsui/app/workspace/virtual-keys/views/virtualKeyDetailsSheet.tsxui/app/workspace/virtual-keys/views/virtualKeySheet.tsxui/app/workspace/virtual-keys/views/virtualKeysTable.tsxui/components/rateLimitDisplay.tsxui/lib/utils/governance.ts
✅ Files skipped from review due to trivial changes (4)
- ui/app/_fallbacks/enterprise/lib/store/apis/accessProfileApi.ts
- ui/app/_fallbacks/enterprise/lib/store/apis/virtualKeyUsersApi.ts
- ui/app/_fallbacks/enterprise/lib/types/accessProfile.ts
- ui/app/_fallbacks/enterprise/lib/types/user.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- ui/app/workspace/virtual-keys/views/virtualKeysTable.tsx
- ui/lib/utils/governance.ts
- ui/app/workspace/virtual-keys/hooks/useVirtualKeyUsage.ts
danpiths
force-pushed
the
feature/04-16-feat_show_live_access-profile-backed_usage_in_virtual_key_views
branch
from
3913095 to
1a2e6e1
Compare
danpiths
force-pushed
the
feature/04-16-feat_consolidate_user_identity_context_key_and_propagate_user_name
branch
from
af73b47 to
529faf5
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
ui/app/workspace/virtual-keys/views/virtualKeyDetailsSheet.tsx (1)
15-15: Prefer the@/alias for this hook import.This file already uses project aliases elsewhere; switching this one off the relative path keeps the UI import style consistent and avoids fragile
../traversal.Proposed fix
-import { useVirtualKeyUsage } from "../hooks/useVirtualKeyUsage"; +import { useVirtualKeyUsage } from "@/app/workspace/virtual-keys/hooks/useVirtualKeyUsage";Based on learnings: In the UI codebase, prefer alias imports using
@/... over relative imports.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/app/workspace/virtual-keys/views/virtualKeyDetailsSheet.tsx` at line 15, Replace the relative import of the hook in virtualKeyDetailsSheet.tsx: update the import of useVirtualKeyUsage to use the project alias (starting with "@/") instead of "../hooks/useVirtualKeyUsage" so the file follows the same alias-based import style used elsewhere (i.e., import useVirtualKeyUsage from "@/.../useVirtualKeyUsage").
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ui/app/workspace/virtual-keys/hooks/useVirtualKeyUsage.ts`:
- Around line 19-41: Add an explicit ownership-loading flag so callers can treat
ownership as unknown until queries resolve: update useVirtualKeyUsage to return
a new boolean (e.g., isOwnershipLoading) and compute it from the query
loading/fetching flags and undefined data from useGetVirtualKeyUsersQuery and
useGetUserAccessProfilesQuery (and account for the skip logic via
managingUserId). While isOwnershipLoading is true, keep
managingProfile/isManagedByProfile as-is or undefined so callers can avoid
treating the VK as unowned; update the hook's return signature to include
isOwnershipLoading and adjust callers to hold off on destructive actions until
that flag is false.
In `@ui/app/workspace/virtual-keys/views/virtualKeySheet.tsx`:
- Around line 409-416: The managed-key update currently uses "description:
data.description || undefined" which omits falsy values and prevents clearing
the description; change the payload logic in the isManagedByProfile branch
(where updateVirtualKey is called with vkId and data) to send an explicit null
when the user cleared the field (e.g. description: data.description === '' ?
null : data.description ?? undefined) so the backend receives an explicit clear
intent, while preserving the current handling for name (data.name || undefined)
or using the null-aware pattern if desired.
In `@ui/lib/utils/governance.ts`:
- Around line 71-76: The UI currently hides unmapped rate-limit durations by
using shortDurationLabels[...] ?? "" which yields strings like "10K tokens"
without the window; update the suffix logic used for both token and request
blocks (where shortDurationLabels is referenced and lines.push uses
formatCompactNumber for token_max_limit and request_max_limit) to fall back to
the raw duration string when no mapping exists (e.g., use
rateLimits.token_reset_duration or rateLimits.request_reset_duration as the
fallback) so the displayed text becomes "10K tokens/30s" or "10K req/1Y" instead
of omitting the window entirely.
---
Nitpick comments:
In `@ui/app/workspace/virtual-keys/views/virtualKeyDetailsSheet.tsx`:
- Line 15: Replace the relative import of the hook in
virtualKeyDetailsSheet.tsx: update the import of useVirtualKeyUsage to use the
project alias (starting with "@/") instead of "../hooks/useVirtualKeyUsage" so
the file follows the same alias-based import style used elsewhere (i.e., import
useVirtualKeyUsage from "@/.../useVirtualKeyUsage").
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 19e612a7-79cd-4a79-8558-72ad2b625f15
📒 Files selected for processing (11)
ui/app/_fallbacks/enterprise/lib/store/apis/accessProfileApi.tsui/app/_fallbacks/enterprise/lib/store/apis/virtualKeyUsersApi.tsui/app/_fallbacks/enterprise/lib/types/accessProfile.tsui/app/_fallbacks/enterprise/lib/types/user.tsui/app/workspace/dashboard/components/modelRankingsTab.tsxui/app/workspace/virtual-keys/hooks/useVirtualKeyUsage.tsui/app/workspace/virtual-keys/views/virtualKeyDetailsSheet.tsxui/app/workspace/virtual-keys/views/virtualKeySheet.tsxui/app/workspace/virtual-keys/views/virtualKeysTable.tsxui/components/rateLimitDisplay.tsxui/lib/utils/governance.ts
✅ Files skipped from review due to trivial changes (4)
- ui/app/_fallbacks/enterprise/lib/store/apis/accessProfileApi.ts
- ui/app/_fallbacks/enterprise/lib/store/apis/virtualKeyUsersApi.ts
- ui/app/_fallbacks/enterprise/lib/types/accessProfile.ts
- ui/app/_fallbacks/enterprise/lib/types/user.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- ui/app/workspace/dashboard/components/modelRankingsTab.tsx
- ui/components/rateLimitDisplay.tsx
- ui/app/workspace/virtual-keys/views/virtualKeysTable.tsx
danpiths
force-pushed
the
feature/04-16-feat_show_live_access-profile-backed_usage_in_virtual_key_views
branch
from
1a2e6e1 to
ab41166
Compare
danpiths
force-pushed
the
feature/04-16-feat_consolidate_user_identity_context_key_and_propagate_user_name
branch
from
529faf5 to
313d58b
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
ui/app/workspace/virtual-keys/views/virtualKeysTable.tsx (1)
54-73: CSV export shows raw VK values, not AP-managed live usage.The
virtualKeysToCSVfunction reads directly fromvk.budgets/vk.rate_limit, while the table cells now display AP-managed values viauseVirtualKeyUsage. Users exporting CSV may see different numbers than displayed in the UI for AP-managed keys.If intentional (export captures VK-owned governance, not live AP counters), consider adding a note in the export dialog. Otherwise, this may need alignment.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/app/workspace/virtual-keys/views/virtualKeysTable.tsx` around lines 54 - 73, virtualKeysToCSV currently reads static governance fields (vk.budgets, vk.rate_limit) which causes exported CSV to diverge from the AP-managed live values shown by useVirtualKeyUsage; update virtualKeysToCSV to accept (or be passed) the AP-managed live usage data (e.g., a usage map produced by useVirtualKeyUsage) and use those live counters for Budget Spent / Status / Budget Limit columns instead of vk.budgets/vk.rate_limit, falling back to vk fields if live data is unavailable; alternatively, if the intent is to export governance-only values, add a clear descriptive flag/parameter and surface that in the export UI so callers choose which dataset to export.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ui/app/workspace/virtual-keys/views/virtualKeyDetailsSheet.tsx`:
- Around line 217-226: The provider-budget and rate-limit blocks in
virtualKeyDetailsSheet.tsx currently call formatDistanceToNow(new
Date(b.last_reset)) without guarding for null/empty last_reset, causing "Invalid
Date" errors; update the provider budget and rate-limit renderers (the blocks
that render b.last_reset and similar provider.rate_limit.last_reset) to use the
same conditional pattern as the VK-level rate-limit section: check that
last_reset is present/truthy before calling new Date(...) and
formatDistanceToNow, and otherwise render a placeholder like "never" or "-" so
the UI never passes an invalid date into formatDistanceToNow (ensure you update
every occurrence where b.last_reset or provider.rate_limit.last_reset is used).
- Around line 344-357: The budget render block passes b.last_reset directly into
new Date() which can be empty/undefined and yields "Invalid Date"; update the
displayBudgets map in VirtualKeyDetailsSheet so the "Last reset" span only calls
formatDistanceToNow(new Date(...)) when b.last_reset is present and parses to a
valid date (e.g., Date.parse(b.last_reset) not NaN), otherwise render a safe
fallback like "never" or an empty string; locate the displayBudgets mapping and
change the b.last_reset usage inside the span to perform this guard before
calling formatDistanceToNow.
---
Nitpick comments:
In `@ui/app/workspace/virtual-keys/views/virtualKeysTable.tsx`:
- Around line 54-73: virtualKeysToCSV currently reads static governance fields
(vk.budgets, vk.rate_limit) which causes exported CSV to diverge from the
AP-managed live values shown by useVirtualKeyUsage; update virtualKeysToCSV to
accept (or be passed) the AP-managed live usage data (e.g., a usage map produced
by useVirtualKeyUsage) and use those live counters for Budget Spent / Status /
Budget Limit columns instead of vk.budgets/vk.rate_limit, falling back to vk
fields if live data is unavailable; alternatively, if the intent is to export
governance-only values, add a clear descriptive flag/parameter and surface that
in the export UI so callers choose which dataset to export.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1b8523dc-e3e5-45a5-b75d-c45ad1f1a436
📒 Files selected for processing (11)
ui/app/_fallbacks/enterprise/lib/store/apis/accessProfileApi.tsui/app/_fallbacks/enterprise/lib/store/apis/virtualKeyUsersApi.tsui/app/_fallbacks/enterprise/lib/types/accessProfile.tsui/app/_fallbacks/enterprise/lib/types/user.tsui/app/workspace/dashboard/components/modelRankingsTab.tsxui/app/workspace/virtual-keys/hooks/useVirtualKeyUsage.tsui/app/workspace/virtual-keys/views/virtualKeyDetailsSheet.tsxui/app/workspace/virtual-keys/views/virtualKeySheet.tsxui/app/workspace/virtual-keys/views/virtualKeysTable.tsxui/components/rateLimitDisplay.tsxui/lib/utils/governance.ts
✅ Files skipped from review due to trivial changes (5)
- ui/app/workspace/dashboard/components/modelRankingsTab.tsx
- ui/app/_fallbacks/enterprise/lib/store/apis/accessProfileApi.ts
- ui/app/_fallbacks/enterprise/lib/store/apis/virtualKeyUsersApi.ts
- ui/app/_fallbacks/enterprise/lib/types/accessProfile.ts
- ui/app/_fallbacks/enterprise/lib/types/user.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- ui/components/rateLimitDisplay.tsx
- ui/app/workspace/virtual-keys/views/virtualKeySheet.tsx
danpiths
force-pushed
the
feature/04-16-feat_show_live_access-profile-backed_usage_in_virtual_key_views
branch
from
ab41166 to
c8e88fc
Compare
Contributor
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 (1)
ui/app/workspace/virtual-keys/views/virtualKeyDetailsSheet.tsx (1)
47-71:⚠️ Potential issue | 🟡 MinorUse hook-provided exhaustion state to avoid drift and zero-limit edge bugs.
isExhaustedis recomputed locally with truthy checks (token_current_usage && token_max_limit), which diverges from the hook's canonical!= nulllogic and can misclassify zero-valued usage as non-exhausted.Suggested fix
- const { assignedUsers, isManagedByProfile, managingProfile, hasApRateLimit, displayBudgets, displayRateLimit } = + const { assignedUsers, isManagedByProfile, managingProfile, hasApRateLimit, displayBudgets, displayRateLimit, isExhausted } = useVirtualKeyUsage(virtualKey); - const isExhausted = - // Budget exhausted (AP-mirrored when managed, VK-own otherwise) - displayBudgets?.some((b) => b.current_usage >= b.max_limit) || - // Rate limits exhausted - (displayRateLimit?.token_current_usage && - displayRateLimit?.token_max_limit && - displayRateLimit.token_current_usage >= displayRateLimit.token_max_limit) || - (displayRateLimit?.request_current_usage && - displayRateLimit?.request_max_limit && - displayRateLimit.request_current_usage >= displayRateLimit.request_max_limit);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/app/workspace/virtual-keys/views/virtualKeyDetailsSheet.tsx` around lines 47 - 71, The local isExhausted recomputes exhaustion with truthy checks (which misclassifies zero values) instead of using the canonical exhaustion value from the hook; replace the manual computation by consuming the exhaustion state returned by useVirtualKeyUsage (e.g., use the hook's isExhausted or equivalent property) and remove the local displayBudgets/displayRateLimit truthy checks; ensure you fall back to false if the hook value is undefined so the component uses the hook's canonical null/!= null semantics rather than the current token_current_usage && token_max_limit style.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ui/app/workspace/virtual-keys/views/virtualKeyDetailsSheet.tsx`:
- Around line 242-275: The UI hides rate-limit sections when max limit values
are 0 because it uses truthy checks; change all conditionals that test
config.rate_limit.token_max_limit and config.rate_limit.request_max_limit to
nullish presence checks (e.g., compare against null/undefined using != null) so
zero is treated as a valid limit, and update the "No rate limits configured"
rendering logic to only show when both token_max_limit and request_max_limit are
null/undefined (not when they are 0). Locate and update conditionals around
config.rate_limit.token_max_limit, config.rate_limit.request_max_limit and the
related "No rate limits configured" branches (UsageLine, parseResetPeriod,
token_last_reset/request_last_reset) to use != null checks.
---
Outside diff comments:
In `@ui/app/workspace/virtual-keys/views/virtualKeyDetailsSheet.tsx`:
- Around line 47-71: The local isExhausted recomputes exhaustion with truthy
checks (which misclassifies zero values) instead of using the canonical
exhaustion value from the hook; replace the manual computation by consuming the
exhaustion state returned by useVirtualKeyUsage (e.g., use the hook's
isExhausted or equivalent property) and remove the local
displayBudgets/displayRateLimit truthy checks; ensure you fall back to false if
the hook value is undefined so the component uses the hook's canonical null/!=
null semantics rather than the current token_current_usage && token_max_limit
style.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3e810be1-8d83-40ff-8cdb-5bdef6182fbf
📒 Files selected for processing (11)
ui/app/_fallbacks/enterprise/lib/store/apis/accessProfileApi.tsui/app/_fallbacks/enterprise/lib/store/apis/virtualKeyUsersApi.tsui/app/_fallbacks/enterprise/lib/types/accessProfile.tsui/app/_fallbacks/enterprise/lib/types/user.tsui/app/workspace/dashboard/components/modelRankingsTab.tsxui/app/workspace/virtual-keys/hooks/useVirtualKeyUsage.tsui/app/workspace/virtual-keys/views/virtualKeyDetailsSheet.tsxui/app/workspace/virtual-keys/views/virtualKeySheet.tsxui/app/workspace/virtual-keys/views/virtualKeysTable.tsxui/components/rateLimitDisplay.tsxui/lib/utils/governance.ts
✅ Files skipped from review due to trivial changes (4)
- ui/app/_fallbacks/enterprise/lib/store/apis/virtualKeyUsersApi.ts
- ui/app/_fallbacks/enterprise/lib/types/accessProfile.ts
- ui/components/rateLimitDisplay.tsx
- ui/app/_fallbacks/enterprise/lib/types/user.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- ui/app/workspace/dashboard/components/modelRankingsTab.tsx
- ui/app/_fallbacks/enterprise/lib/store/apis/accessProfileApi.ts
- ui/app/workspace/virtual-keys/hooks/useVirtualKeyUsage.ts
- ui/app/workspace/virtual-keys/views/virtualKeySheet.tsx
- ui/app/workspace/virtual-keys/views/virtualKeysTable.tsx
danpiths
force-pushed
the
feature/04-16-feat_show_live_access-profile-backed_usage_in_virtual_key_views
branch
from
c8e88fc to
1e7027c
Compare
danpiths
force-pushed
the
feature/04-16-feat_show_live_access-profile-backed_usage_in_virtual_key_views
branch
from
1e7027c to
aefb353
Compare
danpiths
force-pushed
the
feature/04-16-feat_consolidate_user_identity_context_key_and_propagate_user_name
branch
from
313d58b to
3bd057e
Compare
Collaborator
Author
|
@coderabbitai resolve |
Contributor
✅ Actions performedComments resolved and changes approved. |
akshaydeo
previously approved these changes
Apr 17, 2026
Contributor
Merge activity
|
akshaydeo
changed the base branch from
feature/04-16-feat_consolidate_user_identity_context_key_and_propagate_user_name
to
graphite-base/2761
akshaydeo
dismissed stale reviews from coderabbitai[bot] and themself
The base branch was changed.
akshaydeo
approved these changes
Apr 17, 2026
akshaydeo
approved these changes
Apr 17, 2026
akshaydeo
deleted the
feature/04-16-feat_show_live_access-profile-backed_usage_in_virtual_key_views
branch
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
3 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
Reworks the virtual-keys table and details sheet to reflect live usage when a
VK is managed by an access profile, and extracts the supporting rate-limit
display + number formatting into shared pieces that the dashboard model
rankings tab also adopts. When a VK is attached to users via an access
profile, the governance plugin tracks usage on the AP rather than the VK (to
avoid double-counting); the UI now mirrors that — polling the managing AP
every 5s so bars update without a full page refresh — and soft-locks the VK
edit form to name/description only for managed VKs.
The "is this VK AP-managed?" signal is resolved strictly: a VK is treated as
managed only when an access profile explicitly lists it in
virtual_key_ids(no fallback to first-active / first AP), so multi-AP setups can't
misattribute one profile's budgets to another's VKs. Row status, budget
cells, and rate-limit cells all derive exhaustion from that same source, so
the
Active/Exhaustedbadge stays in lockstep with the bars next to it.Changes
Shared:
ui/lib/utils/governance.ts: newformatCompactNumber(n)(e.g.10_000 → "10K",1_500_000 → "1.5M") andformatRateLimitLines(rl)helpers.
formatCompactNumberis backed byIntl.NumberFormatcompactnotation so trailing zeros are dropped (
10_000 → "10K", not"10.0K")and boundary values promote correctly (
999_950 → "1M", not"1000.0K").ui/components/rateLimitDisplay.tsx(new): shared<RateLimitDisplay>component with compact bars + tooltips, a
limitOnlymode for templateentities (access profiles), and a
compactprop for tight cells.ui/app/workspace/dashboard/components/modelRankingsTab.tsx: drop thelocal
formatNumbercopy in favor of the sharedformatCompactNumber.OSS build support:
ui/app/_fallbacks/enterprise/lib/types/accessProfile.ts,.../types/user.ts,.../store/apis/accessProfileApi.ts,.../store/apis/virtualKeyUsersApi.ts(new): stub types + no-op hooksthat resolve the
@enterprise/*imports used byuseVirtualKeyUsagewhen building OSS. The hooks return
{ data: undefined, isLoading: false }so in OSS the hook never finds a managing AP and transparentlyfalls through to the VK's own budgets / rate limits (identical behavior
to pre-PR OSS). Mirrors the existing
largePayload.tsfallback pattern.Virtual keys:
ui/app/workspace/virtual-keys/hooks/useVirtualKeyUsage.ts(new): resolvesthe managing access profile for a VK via
useGetVirtualKeyUsersQuery+useGetUserAccessProfilesQuery(polled every 5s). The managing profileis matched strictly by
virtual_key_ids.includes(vk.id), andisManagedByProfile = managingProfile !== undefined— a VK withdirect-attached users but no AP relation is not treated as AP-managed.
When profile-managed,
displayBudgets/displayRateLimitmirror the AP'svalues (including empty ones — an AP with no rate limits shows nothing,
instead of leaking the VK's own stale rate limits). When not
profile-managed, the hook returns the VK's own budgets / rate limits as
before. Also exposes
assignedUsersandisExhausted(derived from thesame source) so consumers don't reimplement the logic.
ui/app/workspace/virtual-keys/views/virtualKeysTable.tsx:useVirtualKeyUsage(RTKQuery dedupes the underlying request across the per-row hook call
sites) so row status and the bars beside it never disagree.
VKStatusBadgecomponent derivesActive/Exhausted/Inactivefrom the hook's
isExhausted, so a managed key never shows "Active"next to exhausted-looking bars.
a
<span>withcursor-not-allowedand a tooltip (300ms delay) thatexplains the profile must be detached first — instead of luring the user
into confirm-then-403.
AlertDialogCancel / Confirm buttons carrydata-testid="vk-delete-cancel-${vk.name}"/data-testid="vk-delete-confirm-${vk.name}"per the project'sentity-element-qualifierconvention.ui/app/workspace/virtual-keys/views/virtualKeyDetailsSheet.tsx: newUsageLinecomponent with threshold-colored progress bars, managed-byinfo alert that explicitly calls out name/description as still editable,
"Assigned To" section, and section headers that attribute budgets /
rate limits to
managingProfile.namewhen managed.Last reset {...}timestamps render only when the underlying
token_last_reset/request_last_resetfield is populated — no placeholder "just now" whenthe AP has never reset its counters.
ui/app/workspace/virtual-keys/views/virtualKeySheet.tsx: detectAP-managed VK on edit via the same
useVirtualKeyUsagehook; if managed,submit only name/description updates and wrap the rest of the form in a
disabled
<fieldset>with a lock alert and assigned-users display. Thedetection uses the hook's strict
isManagedByProfileso a VK withdirect-attached (non-AP) users still gets the full edit form and its edits
aren't silently dropped by the restricted submit path.
Type of change
Affected areas
How to test
Functional:
user. The user gets a VK minted through the profile.
budget and rate-limit cells should update roughly every 5s without
refreshing the page.
name/description remain editable, and the Budget/Rate Limits sections
should be annotated "(from {access profile name})".
not-allowed cursor and show a tooltip (after ~300ms hover).
be locked by a visible
<fieldset disabled>with an accompanying alert.render the VK's own counters and the full edit form is unlocked.
10K,1.5Mstill rendercorrectly (now from the shared
formatCompactNumber).Screenshots/Recordings
UI change — screenshots/recordings to be attached in the PR description.
Breaking changes
Related issues
N/A
Security considerations
No auth, secret, or PII surface changes. The new hook only reads from
already-authorized RTK Query endpoints.
Checklist
docs/contributing/README.mdand followed the guidelines