Skip to content

Conversation

@Light2Dark
Copy link
Contributor

@Light2Dark Light2Dark commented Jan 20, 2026

📝 Summary

Fixes https://discord.com/channels/1059888774789730424/1462883649333624872

Whenever the first AI provider config is added, we try to infer which chat and edit model to set.

Also fixes some config (eg. azure base url) not being saved.

  • Remove defaultValue because it changes uncontrolled to controlled
  • Move disabled to a different component to prevent setState calls which prevent updating

🔍 Description of Changes

📋 Checklist

  • I have read the contributor guidelines.
  • For large changes, or changes that affect the public API: this change was discussed or approved through an issue, on Discord, or the community discussions (Please provide a link if applicable).
  • Tests have been added for the changes made.
  • Documentation has been updated where applicable, including docstrings for API changes.
  • Pull request title is a good summary of the changes - it will be used in the release notes.
@vercel
Copy link

vercel bot commented Jan 20, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Review Updated (UTC)
marimo-docs Ready Ready Preview, Comment Jan 22, 2026 5:02pm

Request Review

@Light2Dark Light2Dark added the enhancement New feature or request label Jan 20, 2026
Copy link
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 implements automatic model selection when AI provider credentials are configured, improving the onboarding experience. When users add their first AI provider configuration, the system now intelligently selects appropriate chat and edit models. Additionally, the PR fixes an issue where certain configuration fields (like Azure base URL) were not being saved properly due to incorrect placement of the disabled prop.

Changes:

  • Added logic to auto-populate chat and edit models based on configured AI provider credentials
  • Refactored model registry to track default models per provider
  • Fixed form field handling by moving disabled prop to actual input elements instead of FormField wrapper

Reviewed changes

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

Show a summary per file
File Description
frontend/src/core/ai/model-registry.ts Refactored to return both model map and default model per provider; added logic to track first chat/edit-capable model for each provider
frontend/src/components/chat/chat-panel.tsx Improved naming and messaging for unconfigured AI state
frontend/src/components/app-config/user-config-form.tsx Added setAiModels function to auto-populate models when credentials are saved
frontend/src/components/app-config/ai-config.tsx Fixed form field handling by moving disabled prop to inputs; changed helper to always return string for controlled components
frontend/src/components/ai/ai-utils.ts New utility functions for detecting configured providers and recommending default models
frontend/src/components/ai/__tests__/ai-utils.test.ts Comprehensive test coverage for new AI utility functions

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@Light2Dark Light2Dark marked this pull request as ready for review January 20, 2026 16:45
@Light2Dark Light2Dark requested a review from manzt as a code owner January 20, 2026 16:45
mscolnick
mscolnick previously approved these changes Jan 22, 2026
## 📝 Summary

<!--
Provide a concise summary of what this pull request is addressing.

If this PR closes any issues, list them here by number (e.g., Closes
#123).
-->
Fixes #7915 . 

There's a bug with custom provider config. When editing a field (e.g.,
base_url), the API key gets lost. This happens because:
1. Frontend sends only dirty fields (getDirtyValues)
2. custom_providers is in `replace_paths`, so the partial update
replaces the entire object

`replace_paths` was added because deep merge can't delete providers -
omitting a key means "don't change" not "delete".

I was iterating through some solutions
Option 1: Sentinel values in deep_merge
- Remove custom_providers from replace_paths
- Detect ******** during merge → preserve existing secret
- Use "__DELETED__" → remove the key

Option 2: Frontend sends complete object
- Keep replace_paths behavior
- Frontend always sends full object when on replace_paths

This PR is option 2, both options feel hacky.

## 🔍 Description of Changes

<!--
Detail the specific changes made in this pull request. Explain the
problem addressed and how it was resolved. If applicable, provide before
and after comparisons, screenshots, or any relevant details to help
reviewers understand the changes easily.
-->

## 📋 Checklist

- [x] I have read the [contributor
guidelines](https://github.com/marimo-team/marimo/blob/main/CONTRIBUTING.md).
- [ ] For large changes, or changes that affect the public API: this
change was discussed or approved through an issue, on
[Discord](https://marimo.io/discord?ref=pr), or the community
[discussions](https://github.com/marimo-team/marimo/discussions) (Please
provide a link if applicable).
- [x] Tests have been added for the changes made.
- [ ] Documentation has been updated where applicable, including
docstrings for API changes.
- [x] Pull request title is a good summary of the changes - it will be
used in the [release
notes](https://github.com/marimo-team/marimo/releases).
@Light2Dark Light2Dark added the bash-focus Area to focus on during release bug bash label Jan 22, 2026
@mscolnick mscolnick merged commit b96df00 into main Jan 22, 2026
43 of 49 checks passed
@mscolnick mscolnick deleted the sham/smartly-set-ai-models branch January 22, 2026 18:06
botterYosuke pushed a commit to botterYosuke/marimo that referenced this pull request Jan 23, 2026
## 📝 Summary

<!--
Provide a concise summary of what this pull request is addressing.

If this PR closes any issues, list them here by number (e.g., Closes
marimo-team#123).
-->
Fixes
https://discord.com/channels/1059888774789730424/1462883649333624872

Whenever the **first AI provider config** is added, we try to infer
which chat and edit model to set.

Also fixes some config (eg. azure base url) not being saved.
- Remove defaultValue because it changes uncontrolled to controlled
- Move `disabled` to a different component to prevent setState calls
which prevent updating

## 🔍 Description of Changes

<!--
Detail the specific changes made in this pull request. Explain the
problem addressed and how it was resolved. If applicable, provide before
and after comparisons, screenshots, or any relevant details to help
reviewers understand the changes easily.
-->

## 📋 Checklist

- [x] I have read the [contributor
guidelines](https://github.com/marimo-team/marimo/blob/main/CONTRIBUTING.md).
- [ ] For large changes, or changes that affect the public API: this
change was discussed or approved through an issue, on
[Discord](https://marimo.io/discord?ref=pr), or the community
[discussions](https://github.com/marimo-team/marimo/discussions) (Please
provide a link if applicable).
- [x] Tests have been added for the changes made.
- [ ] Documentation has been updated where applicable, including
docstrings for API changes.
- [x] Pull request title is a good summary of the changes - it will be
used in the [release
notes](https://github.com/marimo-team/marimo/releases).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bash-focus Area to focus on during release bug bash enhancement New feature or request

3 participants