Repository navigation
UI Consistency: 1 issue found
apps/web/src/state/accountLimits.ts — the new per-row environment caption reads presentation.entry.target.label raw (?? null is dead: the schema field is a required, untrimmed Schema.String). This bypasses the app-wide environment display-label contract used by the command palette and environment pickers (resolveEnvironmentOptionLabel) and by the launch popover (deriveEnvironmentDisplayLabel), so a blank label renders a dangling Codex A · separator and the primary environment captions with its raw catalog label (possibly the account name or a generic "local") instead of the shared "This device" label. Inline comment posted with the suggested fix.
No other findings: the two issues flagged in the prior run are resolved at this head — legacyInstanceIdFor is now exported from state/accountLimits.ts and reused by rowKey, and instance captions resolve through getProviderInstanceEntry (the shared display-name contract).
Details
Note
Your check run agent prompt is: .macroscope/check-run-agents/ui-consistency.md
More information about how Check Run Agents work can be found in our Help Center.
Scope reviewed
apps/web/src/components/usage/AccountLimits.tsxapps/web/src/state/accountLimits.tsapps/web/src/state/accountLimits.test.ts
Server, contracts, and lockfile changes were read as context only (outside this check's include patterns).
Checks performed and cleared
- Shared primitives: the extracted
HoverWindowRow/SectionWindowRow/RowCaptionare data-viz rows and captions, not reconstructions ofButton,Input,Select, orMenuprimitives; no primitive geometry, focus ring, or hit target is overridden at the call site. - Tailwind ownership: all new presentation is static owner-local Tailwind in the owning TSX module; no global CSS, theme variable, cascade layer, or variant change, so no emitted-CSS or
@variantconcerns arise. - Layout preservation: the single-row path keeps byte-identical markup to the previous single-account rendering (
only !== undefinedbranch), preserving the hover card'sw-64geometry and the section'ssm:grid-cols-2grid. Addedtruncate+titleon window labels is safe under flex (overflow: hiddenresolvesmin-width: autoto 0) and pairs the clipped text with an accessible tooltip. - Empty/loading states:
rows.length === 0 && isSettlingstill gates "Loading…" versus "No limit data yet", and per-row empty states reuse the same typography as the existing single-row state. - React keys:
rowKeyis${environmentId}:${instanceId}, and the merge keeps at most one row per (environment, provider, instance), so keys are unique. - Consumers:
useAccountLimits's only other consumer (components/usage/UsagePage.tsx) usesrefreshand is unaffected by thesnapshots→byProvidershape change.
Verification gates suggested for the author
- Typecheck plus the new focused tests (
apps/web/src/state/accountLimits.test.ts) cover the merge/labeling contract; a real-app screenshot of the multi-instance Limits strip and hover card (light and dark) would confirm the captioned multi-row layout, including the blank/primary environment-label cases called out above.
🔍 Agent activity — 1 tool call
💭 Looking at the current state of those two files to verify the prior issues are resolved—the legacyInstanceIdFor export and getProviderInstanceEntry usage both check out. Now I'm examining the UI for consistency across the component. Looking at the UI consistency across these extracted components, I'm checking that the label spans now have proper truncation with
truncate,whitespace-nowrap, andtitleattributes to handle long provider slugs—a solid improvement over the previous fixed width approach. For the empty state, I need to verify the condition that renders "No limit data yet" handles both single and multi-row cases correctly. In the multi-row layout, each row group shows its own caption withSnapshotAgerather than displaying it once at the provider header level, which keeps the age information contextually grouped. TheRowCaptioncomponent uses flexbox withgap-1.5and passes through a className for styling, withSnapshotAgesafely returning null when fresh. For truncation to work properly on the caption span, I'm confirming that the flex container's defaultmin-width: autobehavior gets overridden by theoverflow: hiddenfromtruncate, allowing the text to actually shrink and ellipsis as intended. Now I need to verify thatgetProviderInstanceEntryexists with the right signature, and check that the properties likepresentation.entry.target.labelandpresentation.serverConfigare actually available on the objects being used. Looking at the UI consistency issues more carefully. ThereportingEnvironmentscount determines whether environment labels appear in captions, which could lead to inconsistent labeling if a provider's data comes from just one environment even when multiple environments reported overall. There's also a potential bug where the provider header stops displaying the Snapshot column in the multi-row layout. The empty-state logic handles cases where rows exist but have no window data, showing per-row messages appropriately. The flex gap spacing between multi-row groups matches the gap between individual rows, so visual separation between groups might be subtle but it's purely aesthetic. Thetitleattribute on window labels is a helpful addition for truncated text. One thing to flag: thewindowparameter inHoverWindowRowshadows the globalwindowobject, which could cause issues if the code tries to access global window methods likesetInterval. I'm noticing a discrepancy between what grep is showing me and what I see when viewing the file at commit a8bf420. The line numbers don't match — the import statement and atom definitions appear at different locations than expected. I'm trying to figure out whether grep is actually running against the target branch HEAD or something else, since the default should be the commit under review, but the results suggest otherwise. The older commit had the issue, but the current head resolves it. Looking at the UI file more carefully, I'm checking for actual regressions: theSnapshotAgeheader appears only when there's a single row, and switches to per-row captions when there are multiple rows. For multi-row cases, the age component sits in a flex container with right alignment and shrinking, though the caption's truncation behavior needs closer inspection. I'm also verifying the window label styling — it's usingw-9 shrink-0 truncate whitespace-nowrap, which has some redundancy sincetruncatealready handles whitespace, but that's harmless. The labels get a title tooltip for overflow, which handles cases like novel provider slugs from server tests. Everything else checks out: static Tailwind only, no global CSS changes, and the color tones for usage are pre-existing. There's something about the em state condition I should flag though — when there are multiple rows and every one has zero windows, the header shows no age and each group displays the caption plus "No limit data yet", which is fine since "Loading…" only appears when there are no rows at all. The single-row empty case also works correctly by design. The hover card's fixed width of 256px should accommodate the caption and window rows without clipping issues, and theSnapshotAgein the row group isn't duplicated since the provider block above shows nothing. For legacy snapshots without provider configuration, the instanceLabel falls back to raw IDs like "codex" or "claudeAgent" which display as captions in multi-row mode — acceptable for those edge cases. The client-side sorting bylocaleCompareon instanceLabel is standard practice for display purposes and doesn't conflict with the server's decision to avoid locale-dependent ordering for the summary itself. There's a potential scenario where environment captions could be redundant: when only one environment has answered but multiple rows exist from that same environment, each gets labeled unnecessarily. More complex is the case where one environment has answered while another is still pending — if the pending environment later responds, the caption logic might need to handle the transition gracefully. I'm also checking for React key collisions by using the combination of environment ID and instance ID, which should be unique within a provider's rows. Window keys use the window ID directly, which could theoretically duplicate if the server sends duplicate IDs, but that's a pre-existing issue. The main thing I want to verify is whether the diff creates any hidden UI problems with the windowHasTraffic filtering — now that windows with 0% and no reset are dropped server-side, a provider with all untouched windows shows "No limit data yet" instead of empty rows, which is intentional. But I need to check what happens in the multi-row case when a snapshot has no windows. Let me look at the.macroscopeconfig to see if there are any repo-specific UI rules or required primitives I should verify, and check if there's a UI docs file. I'm checking whether thetruncateclass on the RowCaption's inner span actually needsmin-w-0to work properly. Sincetruncateappliesoverflow: hidden, that automatically sets the flex item's minimum width to 0 according to the CSS spec, so the truncation should function correctly even without the explicitmin-w-0class. The empty span wrapper withml-auto shrink-0handles spacing when SnapshotAge is null—it pushes the caption text left while staying out of the way. For the multi-row hover card layout, groups are separated by the same gap as internal rows, which makes adjacent accounts slightly harder to distinguish visually, but each group has its own caption providing context, so this is more of an aesthetic consideration than a rule violation. Looking at theRowCaptioncomponent, it requires aclassNameprop where call sites pass typography classes liketext-[10px] text-muted-foreground—that's appropriate since contextual styling stays at the call site. TheHoverWindowRowandSectionWindowRowcomponents are nearly identical except for size tokens; they could arguably be consolidated into one component with asizeprop, but this PR actually improved things by extracting them into named components rather than keeping them inline, so that's not a regression worth flagging. Checking the imports and exports:AccountLimitsRowis used,legacyInstanceIdForis used,ACCOUNT_LIMITS_ACCEPTED_VERSIONSis used in accountLimits.ts, and all the type imports likeProviderInstanceId,ServerProvider,EnvironmentId, andUsageProviderKindare accounted for. The version check withACCOUNT_LIMITS_ACCEPTED_VERSIONS.includes(summary.contractVersion)is fine, and the client correctly handles v1 summaries that fold to a default instance when no instanceId is present. Thetitleattribute on truncated spans follows the standard accessibility pattern, and theenvironmentLabelfallback toenvironmentIdworks as expected. Now I'm wondering about thetarget.labelfield — if it's an empty string rather than null or undefined, the caption could end up showing "Codex A · " with nothing after the separator. The fallback?? row.environmentIdonly catches null values, not empty strings, so I need to check whether the environment target contract allows empty labels or if they're trimmed somewhere. I'm noticing that the code assignsentry.target.labeldirectly toenvironmentLabel, but sincelabelis already typed as a non-nullable string, the nullish coalescing operator?? nullis redundant and will never actually trigger. This means the app should probably be using one of the established label normalization functions likeresolveEnvironmentOptionLabelorderiveEnvironmentDisplayLabelinstead, depending on whether this is for the primary environment or a branch environment. I need to trace how CommandPalette and other components pass runtimeLabel and savedLabel, and understand how the primary environment gets identified so I can validate my fix. I should also look at ProviderUpdateLaunchNotification.environments.ts around lines 60-85, since it handles a similar pattern of taking environments from presentations and applying labels—that's the closest parallel to what accountLimits.ts is doing with its multi-environment list. There's an established contract for how environment labels are resolved across the app—it normalizes empty values to fall back on the environment ID, and for the primary environment it uses "This device" instead of generic labels like "local". The new Limits caption isn't following this pattern though, which means empty catalog labels could render with a dangling separator, and the display won't match what users see elsewhere in the interface. I can fix this by using the sameresolveEnvironmentOptionLabelfunction that the rest of the app uses, checking if we're dealing with the primary environment the same way CommandPalette does with the_tagproperty. Fixing the environment label... There's a potential layering issue though — the resolver lives in components/BranchToolbar.logic.ts, and importing it from state/accountLimits.ts means state is depending on components, which might violate the architecture. I should check how other state modules handle this kind of logic. There's no clean way to import from components into the state layer without breaking the architecture, so I should resolve the environment caption label directly in the AccountLimits component instead—it already receives theenvironmentLabelfrom the row data. If that's not feasible, the alternative is to normalize the labels where the rows are constructed, or at minimum handle empty values and the primary case consistently. I need to give a precise recommendation with the minimal fix required, rather than guessing at what the code should do. The real issue is that the primary environment captions are inconsistent with how every other picker in the app names environments—the ProviderUpdateLaunchNotification explicitly documents that the primary's catalog label can be the account name, and the resolver already handles generic labels. An empty label creates a dangling separator, which is a concrete problem in this newly introduced caption. The?? nullpattern on a non-nullable field suggests the author anticipated this edge case. ThenameEnvironmentgate based onreportingEnvironments > 1isn't actually a bug—it only over-labels when all rows come from one environment, but there's no case where it's needed but missing. I'll skip that and the duplicated HoverWindowRow/SectionWindowRow markup as extraction improvements. The baseline alignment in RowCaption with the nested SnapshotAge span is fine. In AccountLimits.tsx, the multi-row empty state uses inconsistent text sizing between hover and section views—text-[11px]versustext-xs—which I should check if it's intentional or needs alignment. Actually, looking closer, the empty state classes are consistent with the single-row version, so that's not a finding. The real issue is the environment label bypassing the app-wide contract—entry.target.labelis used raw without going throughresolveEnvironmentOptionLabel, which means an empty string slips through as a dangling separator. This is a consistency violation worth flagging. I'll add an inline comment at line 55 on the right side of the diff whereenvironmentLabelis assigned, pointing out that the raw label access breaks the environment display-label contract and allows empty strings to produce malformed captions. Fixing the label resolution... I need to be careful about import layering here — state/ shouldn't import from components/, so either I move the resolver to a shared module or apply it directly inRowCaptionto ensure the caption uses a single contract for the label. Then I'll post this as a GitHub PR review noting the state failure.
1. modify_pr — One finding: the new environment caption bypasses the app-wide environment display-label contract. Details inline.
Agent Credits: 229 credits