Skip to content

refactor(frontend): split api-keys settings page into components - #1065

Merged
lfnovo merged 2 commits into
mainfrom
refactor/frontend-split-api-keys-page
Jul 11, 2026
Merged

refactor(frontend): split api-keys settings page into components#1065
lfnovo merged 2 commits into
mainfrom
refactor/frontend-split-api-keys-page

Conversation

@lfnovo

@lfnovo lfnovo commented Jul 11, 2026

Copy link
Copy Markdown
Owner

Summary

Move-only refactor of frontend/src/app/(dashboard)/settings/api-keys/page.tsx (1,441 lines → 147 lines). No user-facing changes.

Each of the six components that lived inside the page file now has its own file under src/components/settings/ (exported via the existing barrel index.ts), and the provider config tables moved to src/lib/providers.tsx:

  • CredentialFormDialog.tsx
  • DiscoverModelsDialog.tsx
  • DeleteCredentialDialog.tsx
  • CredentialItem.tsx
  • ProviderSection.tsx
  • DefaultModelSelectors.tsx
  • src/lib/providers.tsxALL_PROVIDERS, PROVIDER_DISPLAY_NAMES, PROVIDER_MODALITIES, PROVIDER_DOCS, TYPE_ICONS, TYPE_COLORS, TYPE_COLOR_INACTIVE, TYPE_LABELS, and the ModelType alias (.tsx because TYPE_ICONS holds JSX)

Props, names and behavior are unchanged; prop types were only reshaped into interface XProps to match the existing convention in components/settings/ (e.g. MigrationBanner).

Intentional changes (the only two)

  1. Select dedup: the 47-line default-model Select block was duplicated character-for-character between the primary and advanced grids in DefaultModelSelectors. It is now a single DefaultModelSelect component used by both call sites (advanced passes showDescription).
  2. Fossil removal: dropped the stale comment in CredentialItem defending against a Proxy-based t() that no longer exists, along with the associated testModelLabel/deleteModelLabel caching variables — t() is called directly now.

Test follow-up

The backend cross-check test tests/test_credential_provider_validation.py::TestSupportedProviderMatchesOtherSourcesOfTruth::test_matches_frontend_all_providers_list parses the frontend source for the ALL_PROVIDERS array; it now reads frontend/src/lib/providers.tsx instead of the old page file (same regex, new path).

Verification

Inside frontend/:

  • npm run lint — 0 errors (6 pre-existing warnings, all in untouched files)
  • npm run test — 12 files, 85 tests passed
  • npm run build — production build succeeds

Backend: uv run pytest tests/ — 403 passed.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 10 files

Confidence score: 3/5

  • In frontend/src/components/settings/CredentialFormDialog.tsx, building dialog titles via t(...).replace('{provider}', ...) is brittle and can break when translation strings or placeholder formats change, leading to incorrect or untranslated add/edit credential titles in the UI; switch to the i18n library’s interpolation pattern (or equivalent typed params) before merging to de-risk localization regressions.
Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="frontend/src/components/settings/CredentialFormDialog.tsx">

<violation number="1" location="frontend/src/components/settings/CredentialFormDialog.tsx:123">
P2: Custom agent: **Known caveats**

The dialog titles for editing and adding credentials use manual string replacement on translated text: `t('apiKeys.editConfig').replace('{provider}', ...)`. This is fragile because if a locale defines the string without the literal `{provider}` placeholder, users will see raw placeholder text. Call `t()` with an interpolation argument (e.g., `t('apiKeys.editConfig', { provider: ... })`) so the i18n library resolves the substitution safely across all locales.</violation>
</file>

Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.

Re-trigger cubic

<DialogHeader>
<DialogTitle>
{isEditing
? t('apiKeys.editConfig').replace('{provider}', PROVIDER_DISPLAY_NAMES[provider] || provider)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Custom agent: Known caveats

The dialog titles for editing and adding credentials use manual string replacement on translated text: t('apiKeys.editConfig').replace('{provider}', ...). This is fragile because if a locale defines the string without the literal {provider} placeholder, users will see raw placeholder text. Call t() with an interpolation argument (e.g., t('apiKeys.editConfig', { provider: ... })) so the i18n library resolves the substitution safely across all locales.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/components/settings/CredentialFormDialog.tsx, line 123:

<comment>The dialog titles for editing and adding credentials use manual string replacement on translated text: `t('apiKeys.editConfig').replace('{provider}', ...)`. This is fragile because if a locale defines the string without the literal `{provider}` placeholder, users will see raw placeholder text. Call `t()` with an interpolation argument (e.g., `t('apiKeys.editConfig', { provider: ... })`) so the i18n library resolves the substitution safely across all locales.</comment>

<file context>
@@ -0,0 +1,270 @@
+        <DialogHeader>
+          <DialogTitle>
+            {isEditing
+              ? t('apiKeys.editConfig').replace('{provider}', PROVIDER_DISPLAY_NAMES[provider] || provider)
+              : t('apiKeys.addConfig').replace('{provider}', PROVIDER_DISPLAY_NAMES[provider] || provider)}
+          </DialogTitle>
</file context>

@lfnovo
lfnovo force-pushed the refactor/frontend-split-api-keys-page branch from e80c7d2 to 425c515 Compare July 11, 2026 21:48
@lfnovo

lfnovo commented Jul 11, 2026

Copy link
Copy Markdown
Owner Author

Valid catch, but pre-existing: the .replace('{provider}', ...) pattern was moved verbatim from the old page file (this PR is move-only by design). There are 74 occurrences of this pattern across the app; an upcoming PR converts them all to i18next interpolation (t(key, { provider }) + {{provider}} in the 14 locales) in one codemod pass. Fixing just this file here would split the pattern migration across two PRs.

@lfnovo
lfnovo merged commit f6f7265 into main Jul 11, 2026
10 checks passed
@lfnovo
lfnovo deleted the refactor/frontend-split-api-keys-page branch September 2, 2026 22:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant