Solid state-management logic keeps getting undercut by unfinished edges: silent save failures, unreachable keyboard controls, and accessibility left as an afterthought. The keyboard-inaccessible retry link is the single biggest liability, a critical gap that blocks recovery for real users.

Files Rams reviewed

src/components/server/MobileNavLayout.tsx

src/app/layout.tsx

packages/builtin-tool-lobe-agent/src/client/components/SortableTodoList/index.tsx

src/components/404/index.tsx

src/components/Analytics/index.tsx

src/components/AnimatedCollapsed/index.tsx

src/components/AsyncBoundary/index.tsx

src/components/AsyncError/index.tsx

src/components/AvatarUpload/index.tsx

src/components/BootErrorBoundary/index.tsx

src/components/BrandWatermark/index.tsx

src/components/Cell/index.tsx

src/components/CircleLoader/index.tsx

src/components/CodeEditorPane/index.tsx

src/components/CopyableLabel/index.tsx

src/components/DataStyleModal/index.tsx

src/components/Descriptions/index.tsx

src/components/DotsLoading/index.tsx

src/components/DragUpload/index.tsx

src/components/DragUploadZone/index.tsx

src/components/Error/index.tsx

src/components/ErrorBoundary/index.tsx

src/components/FeatureList/index.tsx

src/components/FileIcon/index.tsx

src/components/FileParsingStatus/index.tsx

src/components/FormAction/index.tsx

src/components/GalleyGrid/index.tsx

src/components/GoBack/index.tsx

src/components/GuideModal/index.tsx

src/components/HighlightNotification/index.tsx

94/100

UX

3 serious

UX·packages/builtin-tool-lobe-agent/src/client/components/SortableTodoList/index.tsx:42Serious

Flush-on-approve and flush-on-unmount give no feedback when save fails

`registerBeforeApprove` awaits `store.getState().saveNow()` and `useUnmount` calls `store.getState().flushSave()` with no catch, no error propagation, and no UI signal if either rejects.

Why it matters

If the save request fails right as the user approves or navigates away, the todo edits are lost with zero indication anything went wrong, so the user believes the work is saved when it isn't.

Fix

Catch save failures at this boundary and surface a recoverable error state or retry path instead of letting the promise reject silently.

const unregister = registerBeforeApprove('sortable-todo-list', async () => {
  await store.getState().saveNow();
});

const unregister = registerBeforeApprove('sortable-todo-list', async () => {
  try {
    await store.getState().saveNow();
  } catch (error) {
    store.getState().setSaveError?.(error);
  }
});

UX·src/components/404/index.tsx:39Serious

'Back to home' forces a hard reload instead of behaving like a link

The default CTA is `<Button type={'primary'} onClick={() => (window.location.href = '/')}>`. It navigates by mutating `window.location.href` rather than rendering as a real anchor.

Why it matters

Without a real href, users can't middle-click or Ctrl-click to open home in a new tab, can't preview the destination in the status bar, and every visit triggers a full page reload instead of a client-side transition.

Fix

Render navigation actions as an anchor or router Link with a real href, styled as the primary button, instead of an onClick redirect.

{extra || (
  <Button type={'primary'} onClick={() => (window.location.href = '/')}>
    {t('notFound.backHome')}
  </Button>
)}

{extra || (
  <Button as={'a'} href={'/'} type={'primary'}>
    {t('notFound.backHome')}
  </Button>
)}

UX·src/components/AsyncBoundary/index.tsx:108Serious

Empty state silently renders a blank screen if the `empty` prop is forgotten

The `isEmpty` branch returns `<Flexbox flex={1} height={'100%'} width={'100%'}>{empty}</Flexbox>` and `empty` is typed as optional. If a call site sets `isEmpty` but never passes `empty`, the boundary renders a full-height empty box with nothing in it: no error, no warning, no fallback.

Why it matters

A blank screen with no CTA or message reads as broken to the user, and because this is a shared boundary every future call site that skips the `empty` prop reproduces the same silent failure with no signal to the developer that they forgot it.

Fix

Add a dev-only warning (or a minimal default empty node) when isEmpty is true but empty is missing, so the gap surfaces at build/dev time instead of in production.

if (isEmpty)
  return (
    <Flexbox flex={1} height={'100%'} width={'100%'}>
      {empty}
    </Flexbox>
  );

if (isEmpty) {
  if (process.env.NODE_ENV !== 'production' && !empty) {
    console.warn('AsyncBoundary: isEmpty is true but no `empty` node was provided.');
  }
  return (
    <Flexbox flex={1} height={'100%'} width={'100%'}>
      {empty}
    </Flexbox>
  );
}

95/100

Accessibility

1 critical1 serious

Accessibility·src/components/AsyncError/index.tsx:120Critical

Retry link in inline and metric variants can't be activated by keyboard

Both the `inline` and `metric` variants build the retry action from `<Text role={'button'} tabIndex={0} onClick={onRetry}>` with no `onKeyDown` handler. Setting `role="button"` and `tabIndex={0}` makes the element focusable but does not give it a native button's Enter/Space activation, the browser only wires that up for real `<button>` elements.

Why it matters

Keyboard-only users can tab to the retry control, see it receive focus, and press Enter with nothing happening. The one recovery path AsyncError offers is unreachable for them, which is worse than showing no retry at all.

Fix

Add an onKeyDown handler firing on Enter and Space for any element with role="button", or replace it with a native <button>.

<Text
  role={'button'}
  style={{ color: cssVar.colorPrimary, cursor: 'pointer' }}
  tabIndex={0}
  onClick={onRetry}
>
  {t('error.retry')}
</Text>

<Text
  role={'button'}
  style={{ color: cssVar.colorPrimary, cursor: 'pointer' }}
  tabIndex={0}
  onClick={onRetry}
  onKeyDown={(e) => {
    if (e.key === 'Enter' || e.key === ' ') {
      e.preventDefault();
      onRetry?.();
    }
  }}
>
  {t('error.retry')}
</Text>

Accessibility·src/components/AsyncBoundary/index.tsx:104Serious

Error block replacing the skeleton isn't announced to assistive tech

The failure branch returns `<AsyncError error={error} variant={errorVariant} onRetry={onRetry} />` directly, with no wrapping `role="alert"` or `aria-live` region at the boundary level. Since `AsyncError` mounts in place of the loading skeleton, a screen reader user tabbed away from the region never hears that the request failed.

Why it matters

Because this is the single shared gate for loading/error/empty/data across the codebase's fetch surfaces, an unannounced failure here means users relying on assistive tech get no indication a request failed and no cue to reach for the Retry action.

Fix

Wrap the error branch's output in a live region (role="alert" or aria-live="assertive") so failures are announced the moment they render.

if (error && !hasSettled) {
  return <AsyncError error={error} variant={errorVariant} onRetry={onRetry} />;
}

if (error && !hasSettled) {
  return (
    <div role="alert">
      <AsyncError error={error} variant={errorVariant} onRetry={onRetry} />
    </div>
  );
}

Want this on your repo?

Rams reviews your next PR automatically and posts inline fix suggestions.

Review my public repo for free

98/100

Craft

1 serious

Craft·src/components/AnimatedCollapsed/index.tsx:44Serious

Two 'as any' casts hide a real type mismatch in the style spread

`...(styles?.collapsed as any)` and `...(styles?.open as any)` both cast away the `CSSProperties` type instead of resolving the conflict with the animated `height`/`width`/`opacity` keys below them.

Why it matters

If a caller passes a `styles.open` object with its own `height` or `opacity`, TypeScript can't warn about the collision because the type checker is disabled at exactly the spot where it matters most, and the bug surfaces later as a silent visual glitch.

Fix

Type the styles prop precisely (e.g. Omit<CSSProperties, 'height' | 'width' | 'opacity'>) instead of casting to any.

collapsed: {
  ...(styles?.collapsed as any),
  height: height?.collapsed ?? 0,
  opacity: 0,
  width: width?.collapsed ?? 0,
},

collapsed: {
  ...(styles?.collapsed as Omit<CSSProperties, 'height' | 'width' | 'opacity'>),
  height: height?.collapsed ?? 0,
  opacity: 0,
  width: width?.collapsed ?? 0,
},

Typography

No issues found

Color

No issues found

Spacing

No issues found

Components

No issues found

Motion

No issues found

Working well

The block comment above the component spells out exactly why loading is checked before error (`SWR keeps the previous error until the revalidation settles`), turning a subtle race condition into a documented, deliberate ordering. That's the right way to protect logic like this from a future refactor that 'simplifies' the precedence back into a bug.
Every color reference in this file goes through `cssVar` tokens (`colorTextTertiary`, `colorBorderSecondary`, `colorPrimary`, etc.) with zero hardcoded hex anywhere, including inline styles. That discipline is what makes a future theme or dark-mode change a one-file edit instead of a grep-and-replace across every error surface.
The `metric` variant deliberately renders a failed marker instead of falling back to `0` or blank when a stat fetch errors, that's the right call: a silent `$0` reads as a confident (and wrong) data point, while a visible failed state tells the user the number simply isn't known yet.
`hasSettled = data !== undefined` is a genuinely careful distinction: it separates 'never loaded' from 'settled empty' so a background revalidation error doesn't blow away a legitimately empty list. Most boundary components collapse these into one flag and lose exactly this case.

Score your own repo with Rams.

Free on public repos. Rams reviews every pull request for accessibility, color, type, spacing, components, motion, UX, and craft, and posts inline fixes.

Review my public repo for free