Skip to content

fix(usage): fold first-review findings

MacroscopeApp / Macroscope - UI Consistency failed Aug 15, 2026 in 5m 56s

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.tsx
  • apps/web/src/state/accountLimits.ts
  • apps/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 / RowCaption are data-viz rows and captions, not reconstructions of Button, Input, Select, or Menu primitives; 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 @variant concerns arise.
  • Layout preservation: the single-row path keeps byte-identical markup to the previous single-account rendering (only !== undefined branch), preserving the hover card's w-64 geometry and the section's sm:grid-cols-2 grid. Added truncate + title on window labels is safe under flex (overflow: hidden resolves min-width: auto to 0) and pairs the clipped text with an accessible tooltip.
  • Empty/loading states: rows.length === 0 && isSettling still gates "Loading…" versus "No limit data yet", and per-row empty states reuse the same typography as the existing single-row state.
  • React keys: rowKey is ${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) uses refresh and is unaffected by the snapshots → byProvider shape 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, and title attributes 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 with SnapshotAge rather than displaying it once at the provider header level, which keeps the age information contextually grouped. The RowCaption component uses flexbox with gap-1.5 and passes through a className for styling, with SnapshotAge safely returning null when fresh. For truncation to work properly on the caption span, I'm confirming that the flex container's default min-width: auto behavior gets overridden by the overflow: hidden from truncate, allowing the text to actually shrink and ellipsis as intended. Now I need to verify that getProviderInstanceEntry exists with the right signature, and check that the properties like presentation.entry.target.label and presentation.serverConfig are actually available on the objects being used. Looking at the UI consistency issues more carefully. The reportingEnvironments count 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. The title attribute on window labels is a helpful addition for truncated text. One thing to flag: the window parameter in HoverWindowRow shadows the global window object, which could cause issues if the code tries to access global window methods like setInterval. 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: the SnapshotAge header 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 using w-9 shrink-0 truncate whitespace-nowrap, which has some redundancy since truncate already 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 the SnapshotAge in 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 by localeCompare on 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 .macroscope config 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 the truncate class on the RowCaption's inner span actually needs min-w-0 to work properly. Since truncate applies overflow: 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 explicit min-w-0 class. The empty span wrapper with ml-auto shrink-0 handles 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 the RowCaption component, it requires a className prop where call sites pass typography classes like text-[10px] text-muted-foreground—that's appropriate since contextual styling stays at the call site. The HoverWindowRow and SectionWindowRow components are nearly identical except for size tokens; they could arguably be consolidated into one component with a size prop, 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: AccountLimitsRow is used, legacyInstanceIdFor is used, ACCOUNT_LIMITS_ACCEPTED_VERSIONS is used in accountLimits.ts, and all the type imports like ProviderInstanceId, ServerProvider, EnvironmentId, and UsageProviderKind are accounted for. The version check with ACCOUNT_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. The title attribute on truncated spans follows the standard accessibility pattern, and the environmentLabel fallback to environmentId works as expected. Now I'm wondering about the target.label field — 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.environmentId only 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 assigns entry.target.label directly to environmentLabel, but since label is already typed as a non-nullable string, the nullish coalescing operator ?? null is redundant and will never actually trigger. This means the app should probably be using one of the established label normalization functions like resolveEnvironmentOptionLabel or deriveEnvironmentDisplayLabel instead, 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 same resolveEnvironmentOptionLabel function that the rest of the app uses, checking if we're dealing with the primary environment the same way CommandPalette does with the _tag property. 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 the environmentLabel from 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 ?? null pattern on a non-nullable field suggests the author anticipated this edge case. The nameEnvironment gate based on reportingEnvironments > 1 isn'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] versus text-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.label is used raw without going through resolveEnvironmentOptionLabel, 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 where environmentLabel is 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 in RowCaption to 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