fix: enable Save button when changing OpenRouter provider routing

[roomote]

Summary

This PR fixes an issue where the Save button in the settings view was not enabling when changing the OpenRouter Provider Routing setting.

Problem

When users changed the OpenRouter Provider Routing in Advanced Settings, the Save button remained disabled because the change was incorrectly treated as an "initial sync" rather than a user modification.

Solution

Modified the setApiConfigurationField function in SettingsView.tsx to exclude openRouterSpecificProvider from the initial sync logic. This ensures that changes to this field properly trigger the change detection and enable the Save button.

Changes

• Updated the initial sync condition to specifically exclude openRouterSpecificProvider
• Added field !== "openRouterSpecificProvider" to the condition check

Testing

• All existing tests pass
• Manually verified that changing the OpenRouter Provider Routing now enables the Save button
• No regression in other settings functionality

Fixes #6655

---

Important

Fixes Save button not enabling when changing OpenRouter Provider Routing by updating setApiConfigurationField logic in SettingsView.tsx.

• Behavior:
  • Fixes issue where Save button in settings view was not enabling when changing openRouterSpecificProvider.
  • Modifies setApiConfigurationField in SettingsView.tsx to exclude openRouterSpecificProvider from initial sync logic.
• Solution:
  • Updated initial sync condition to exclude openRouterSpecificProvider.
  • Added field !== "openRouterSpecificProvider" to condition check.
• Testing:
  • All existing tests pass.
  • Manually verified Save button enables on changing OpenRouter Provider Routing.
  • No regression in other settings functionality.

^{This description was created by for e513a5c. You can customize this summary. It will automatically update as commits are pushed.}

[roomote]

Fixed a bug I created. The irony is not lost on my circuits.

[roomote]

The fix correctly addresses the issue, but I'm wondering if we should consider a more scalable approach for the future? Instead of adding field-specific exceptions, we could maintain a list of fields that should always trigger change detection:

Suggested change

	previousValue === undefined && value !== undefined && field !== "openRouterSpecificProvider"
	// Don't treat initial sync from undefined to a defined value as a user change
	// This prevents the dirty state when the component initializes and auto-syncs the model ID
	// Exception: certain fields should always trigger change detection
	const ALWAYS_TRIGGER_CHANGE_FIELDS = ["openRouterSpecificProvider"]
	const isInitialSync =
	previousValue === undefined && value !== undefined && !ALWAYS_TRIGGER_CHANGE_FIELDS.includes(field)

This would make it easier to add more fields in the future without modifying the condition logic.

[roomote]

Consider adding tests for this component to verify that changing openRouterSpecificProvider properly enables the Save button. This would help prevent regression of this bug and ensure the fix works as expected.

[daniel-lxs]

@roomote-agent this solution isn't scalable, make sure this setting is also detectable when changed but following the already existing behavior for the other ones, push to this PR, do not create a new one

[roomote]

Hi @daniel-lxs, I see your comment. You're right that the current hardcoded exception isn't scalable. I'll investigate the existing patterns for change detection in the settings and implement a more robust solution that follows the established behavior. Working on this now.

[roomote]

I've implemented a more scalable solution that addresses your feedback. Instead of hardcoding exceptions for specific fields, the implementation now:

1. Tracks user-initiated changes: Added a userModifiedFields state that keeps track of which fields have been explicitly modified by user interaction
2. Distinguishes between initial sync and user actions: The setApiConfigurationField function now accepts an isUserAction parameter (defaults to true) to differentiate between programmatic updates and user interactions
3. Smart change detection: Only triggers the save button for:
   • Fields that have been explicitly modified by the user
   • Any subsequent changes to those fields (even programmatic ones)
4. Proper cleanup: Resets the tracking when settings are saved, discarded, or reloaded

This approach eliminates the need for hardcoded field names and will automatically work for any new fields that need similar behavior in the future.

All CI checks have passed ✅

[daniel-lxs]

@roomote-agent This implementation is overly complex for the problem it's trying to solve. The real issue is that we shouldn't need special handling for the openRouterSpecificProvider setting - it should behave like any other setting where changes are detected as changes.

The current approach adds unnecessary complexity:

1. The isUserAction parameter is never actually used with false anywhere in the codebase
2. The userModifiedFields state adds memory overhead and the dependency array could cause performance issues
3. We're treating a symptom rather than the root cause

What needs to be done:

• Investigate why openRouterSpecificProvider requires special handling when other settings don't
• Find and fix the root cause of why this field's changes aren't being detected properly
• Remove the userModifiedFields tracking and isUserAction parameter - they add complexity without solving the underlying issue
• Ensure all settings fields behave consistently without needing field-specific logic

The solution should make the openRouterSpecificProvider field work like every other setting field, not add special case handling for it.

[roomote]

Thanks for the feedback @daniel-lxs! I understand your concerns about the complexity. Let me investigate why openRouterSpecificProvider requires special handling and find the root cause. I'll work on a cleaner solution that makes this field behave consistently with other settings fields.

[roomote]

Hi @daniel-lxs,

I've implemented a cleaner solution based on your feedback. Here's what I did:

Root Cause

The issue was that the Select component displayed a default value ([default]) in the UI but the actual field (openRouterSpecificProvider) remained undefined. When users changed the selection, it went from undefined to a value, which the initial sync logic incorrectly treated as initialization rather than a user change.

Solution

Instead of the complex userModifiedFields tracking, I implemented a simpler approach:

• Added a useEffect hook that initializes openRouterSpecificProvider with the default value when the dropdown becomes visible
• This ensures the field always has a value before user interaction
• All subsequent changes are properly detected as user modifications

Changes Made

1. ✅ Removed userModifiedFields state and all related tracking logic
2. ✅ Removed the isUserAction parameter from setApiConfigurationField
3. ✅ Added clean initialization logic in ApiOptions.tsx using useEffect
4. ✅ All tests pass locally

The solution is much cleaner and makes openRouterSpecificProvider behave consistently with other settings fields without special case handling.

CI checks are currently running. Let me know if you have any questions or need any adjustments!

[daniel-lxs]

I am unable to reproduce this issue, closing for now