Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: Error while selecting providers and models. Anthropic was always the default provider even after disabling it. #693

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

yodaljit
Copy link

[2024-12-13] Provider Settings and Model Selection Fixes

Fixed

  • Fixed infinite update loop in ModelSelector component
  • Fixed file-saver import issues in workbench store
  • Improved provider settings initialization
  • Fixed path handling in browser environment

Changed Files

app/lib/stores/workbench.ts and app/lib/runtime/action-runner.ts

  • Replaced Node.js path utilities with browser-compatible functions:
    function joinPath(...parts: string[]): string {
      return parts.join('/').replace(/\/+/g, '/');
    }
    
    function dirname(path: string): string {
      const normalizedPath = path.replace(/\/+/g, '/').replace(/\/$/, '');
      const lastSlashIndex = normalizedPath.lastIndexOf('/');
      if (lastSlashIndex === -1) {
        return '.';
      }
      const dir = normalizedPath.slice(0, lastSlashIndex);
      return dir || '/';
    }
    
    function basename(path: string): string {
      const normalizedPath = path.replace(/\/+/g, '/').replace(/\/$/, '');
      const lastSlashIndex = normalizedPath.lastIndexOf('/');
      if (lastSlashIndex === -1) {
        return path;
      }
      return normalizedPath.slice(lastSlashIndex + 1);
    }
  • Updated file-saver import to use CommonJS-compatible syntax:
    import FileSaver from 'file-saver';
    const { saveAs } = FileSaver;

app/lib/hooks/useSettings.tsx

  • Removed providers from useEffect dependency array to prevent infinite updates
  • Cookie loading now only happens once on component mount
  • Improved error handling for provider settings parsing

app/components/chat/ModelSelector.tsx

  • Separated provider and model update logic into distinct effects
  • Fixed undefined firstModel variable reference
  • Removed unnecessary dependencies from effect arrays
  • Added better error handling for provider changes
  • Improved model selection logic when providers change

Technical Details

  1. Provider Settings Initialization

    • Provider settings are now properly loaded from cookies on initial mount
    • Removed circular dependency that was causing infinite updates
  2. Model Selection Logic

    • Model selection now happens after provider changes are complete
    • Added safeguards against undefined model states
    • Improved logging for debugging purposes
  3. State Management

    • Better separation of concerns between provider and model state updates
    • More predictable state update flow
    • Reduced unnecessary re-renders
  4. Browser Compatibility

    • Replaced Node.js path utilities with browser-compatible alternatives
    • Added robust path handling functions for join, dirname, and basename operations
    • Improved error handling for browser-specific file operations
    • Normalized path handling across different components

@thecodacus
Copy link
Collaborator

can you break it down to smaller PRs. there are multiple things you are trying to achieve,
I am not able to understand what issue you are trying to resolve.

i can see the listed down fixes on the top. but cannot relate it to an issue that i have observed.
I seen the issue with settings hook, but that is already fixed in other PRs.

@yodaljit
Copy link
Author

The whole PR is only about the LLM models not getting displayed according to the selected provider. Along the way I also improved the path handling in browser environment where node:path was not accessible.

I don't know in which PR the setting hook was resolved because yesterday only I forked the project and immediately got this issue. Even if I selected any other provider, Anthropic was always the current provider and claude LLMs were displayed in the Model selector.

@thecodacus
Copy link
Collaborator

thank you for all the effort you have put in,
can you try the latest branch and confirm there is an issue right now?
so that I can better understand the problem you are trying to fix

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.

2 participants