Skip to content

fix: fixed name field is mandatory issue#4371

Open
Rutetid wants to merge 5 commits intoarchestra-ai:mainfrom
Rutetid:fix/Name-field-is-mandatory-issue
Open

fix: fixed name field is mandatory issue#4371
Rutetid wants to merge 5 commits intoarchestra-ai:mainfrom
Rutetid:fix/Name-field-is-mandatory-issue

Conversation

@Rutetid
Copy link
Copy Markdown
Contributor

@Rutetid Rutetid commented May 5, 2026

Summary

Fixes #4369

Fixes a UX mismatch where the Name field is labeled optional but required to submit.

Changes

  • Aligns Knowledge settings “Add LLM Provider Key” dialog with the optional Name label.
  • Allows submission without Name and falls back to provider name when blank.

Testing

All frontend test and pnpm check:ci passes

Screen Recording

2026-05-05.16-55-49.mp4

Copilot AI review requested due to automatic review settings May 5, 2026 11:27
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes the knowledge settings “Add LLM Provider Key” dialog so the Name field truly behaves as optional, matching the UI and the linked issue. It updates the create flow in the settings page to allow blank names and substitute the selected provider’s display name.

Changes:

  • Removes the Name field from the client-side submit eligibility check.
  • Trims the submitted name before sending it to the create mutation.
  • Falls back to PROVIDER_CONFIG[provider].name when the user leaves Name blank.

Comment thread platform/frontend/src/app/settings/knowledge/page.tsx
@Rutetid
Copy link
Copy Markdown
Contributor Author

Rutetid commented May 5, 2026

added the test pls review @copilot

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Comment thread platform/frontend/src/app/settings/knowledge/page.tsx
Co-authored-by: Copilot <copilot@github.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

@Rutetid
Copy link
Copy Markdown
Contributor Author

Rutetid commented May 5, 2026

my bad, fixed the lint error

@Konstantinov-Innokentii
Copy link
Copy Markdown
Contributor

Konstantinov-Innokentii commented May 5, 2026

If backend requires name, maybe just let's make it required in UI?

@Rutetid
Copy link
Copy Markdown
Contributor Author

Rutetid commented May 6, 2026

If backend requires name, maybe just let's make it required in UI?

@Konstantinov-Innokentii , in this implementation we are falling back to the model name if the user does not provide any name, so we can do that or as you said we can make it mandatory in the UI itself.

Which implementation should we go forward with?

@Konstantinov-Innokentii
Copy link
Copy Markdown
Contributor

Konstantinov-Innokentii commented May 6, 2026

If it's required on backend, let's just make it required on frontend too?

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.

"Name" field in Add LLM Provider Key modal is mandatory despite being labeled optional

4 participants