Skip to content

Conversation

@matthewdiamant
Copy link
Contributor

@matthewdiamant matthewdiamant commented Mar 14, 2025

Add Model modal for Settings v2.

Providers taken from the providers with active keys.
Models is currently just a freeform text field (this is also how the current model input works on Settings v1).

Changing the model will upsert to config.yaml, and then re-initialize the system in the same way Settings v1 does.

This PR also adds a reusable Select UI component that is themed more like the Settings v2 Figma.

Screenshot 2025-03-14 at 5 14 48 PM
Screen.Recording.2025-03-14.at.4.50.28.PM.mov

@matthewdiamant matthewdiamant force-pushed the mdiamant/settings-v2-add-models branch 3 times, most recently from 39a9bb2 to bb33191 Compare March 15, 2025 00:15
Copy link
Contributor

Choose a reason for hiding this comment

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

Noticed that if the list of configured providers is quite long, we the list gets cut off

Screen.Recording.2025-03-16.at.10.49.06.AM.mov

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice catch! I made some changes to Modal to allow for overflow. I checked some of the other modals in Settings V2 and they still look good to me.

image

import Modal from '../../Modal';
import { Button } from '../../ui/button';
import { QUICKSTART_GUIDE_URL } from '../providers/modal/constants';
import { useActiveKeys } from '../../settings/api_keys/ActiveKeysContext';
Copy link
Contributor

Choose a reason for hiding this comment

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

This useActiveKeys will be deprecated, but we could implement something similar in ConfigContext -- currently we return all providers in the getProviders method, which is a list of ProviderDetail objects

One attribute of ProviderDetails is isConfigured, we can always call getProviders here directly and filter out only the the ProviderDetail objects with isConfigured == true to create the drop-down list but maybe you might have an idea on how to handle the state more elegantly to avoid an API call

feel free to do this in a followup PR if easier :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!


<div className="w-full flex flex-col gap-4">
<Select
options={providerOptions}
Copy link
Contributor

Choose a reason for hiding this comment

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

idea: maybe last option in dropdown list is Add Other Provider which, when clicked takes you to the Configure Providers Page -- haven't discussed this with design though, but curious your thoughts about if that would feel more natural to you instead of or in addition to the Configure Providers button

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a super good idea, I'm all for it.

@matthewdiamant matthewdiamant force-pushed the mdiamant/settings-v2-add-models branch from bb33191 to 57c286e Compare March 18, 2025 16:31
@matthewdiamant matthewdiamant force-pushed the mdiamant/settings-v2-add-models branch from 57c286e to 8394808 Compare March 18, 2025 17:10
@matthewdiamant matthewdiamant merged commit 2937be0 into main Mar 18, 2025
6 checks passed
@matthewdiamant matthewdiamant deleted the mdiamant/settings-v2-add-models branch March 18, 2025 23:17
salman1993 added a commit that referenced this pull request Mar 20, 2025
* origin/main: (74 commits)
  config: add optional extension description (#1743)
  docs: add deployment for install link generator (#1737)
  ui: new configure provider flow (#1736)
  Revert "Standardize Radio Button input" (#1758)
  Settings v2 Add Model (#1708)
  fix: use lowercase names for builtin external extensions (#1756)
  chore(release): release version 1.0.15 (#1749)
  docs: goosing around: langfuse blog (#1746)
  feat: update the deny call response (#1741)
  feat: refactor register eval (#1713)
  fix: Goose UI fix typos (#1744)
  feat(google_drive): comment read (#1732)
  feat: build cli workflow  (#1697)
  fix: fix initial model configuration in cli when using toolshim (#1720)
  feat: add basic support for aws bedrock to desktop app (#1271)
  feat(google_drive): add image resizing logic from developer, and use Content::Image (#1735)
  Standardize Radio Button input (#1701)
  ui: tweaks to settings v2 (#1731)
  feat(google_drive): set read/write scope on all commands to use the same token (#1707)
  refactor: clean up log usage (#1704)
  ...
michaelneale added a commit that referenced this pull request Mar 20, 2025
* main:
  fix: check if working directory has changed before asking (#1733)
  extensions: add a display name field (#1759)
  ui: add logs to app (#1760)
  docs: add stdin (#1769)
  config: add optional extension description (#1743)
  docs: add deployment for install link generator (#1737)
  ui: new configure provider flow (#1736)
  Revert "Standardize Radio Button input" (#1758)
  Settings v2 Add Model (#1708)
  fix: use lowercase names for builtin external extensions (#1756)
ahau-square pushed a commit that referenced this pull request May 2, 2025
cbruyndoncx pushed a commit to cbruyndoncx/goose that referenced this pull request Jul 20, 2025
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.

3 participants