ChatCraft week 7: New issue, new provider?

This week on ChatCraft, I am beginning work on a new issue.

Do not require chatcraft users to enter a key to use it #401

tarasglek posted on Feb 04, 2024

If a user does not enter a key, we should use a free model endpoint provided by something like https://taras-free_open_router.web.val.run implemented by https://www.val.town/v/taras/free_open_router . Should still have a banner prompting for user to enter their key while in limited to free models mode. This makes use of a new openrouter feature where free models are singled out with :free suffix

View on GitHub

This issue involves adding a new default provider so users can try it for free. However since the issue is so big I have broken it up into a smaller issue:

Create OpenAiProvider and OpenRouterProvider #478

kliu57 posted on Feb 21, 2024

Subtask of issue #401

• Create providers/OpenAiProvider.ts, providers/OpenRouterAiProvider.ts
• Copy provider-specific methods (for example validateOpenAiApiKey, validateOpenRouterApiKey) to providers/OpenAiProvider.ts, providers/OpenRouterAiProvider.ts
• Test these copied functions on local, but no need to commit code to use them yet

View on GitHub

PR

Created OpenAiProvider and OpenRouterProvider #479

kliu57 posted on Feb 22, 2024

Closes #478

Summary:

• created providers/OpenAiProvider.ts and providers/OpenRouterAiProvider.ts containing provider-specific methods

Code changes in this PR:

src/components/Message/AppMessage/Instructions.tsx:

• the API URL dropdown field now iterates through the list of supportedProviders (missed update from PR 423)

src/components/PreferencesModal.tsx:

• added await and async since fromJSON method is now async

src/lib/ChatCraftProvider.ts:

• modified fromJSON to be async and to return an OpenAiProvider or OpenRouterProvider instead of a ChatCraftProvider object. (OpenAiProvider and OpenRouterProvider are derived/child classes of ChatCraftProvider)
• had to make method async to avoid circular dependency

src/lib/providers/OpenAiProvider.ts:

• created derived/child class of ChatCraftProvider to represent an OpenAi provider
• copied provided-specific methods to this class

src/lib/providers/OpenRouterProvider.ts:

• created derived/child class of ChatCraftProvider to represent an OpenRouter provider
• copied provided-specific methods to this class

View on GitHub

I created providers/OpenAiProvider.ts and providers/OpenRouterAiProvider.ts containing provider-specific methods. These classes are child classes of the parent class ChatCraftProvider which I created last month. Eventually I will also create one called DefaultProvider.ts.

Most of the provider-specific methods I copied over from ai.ts.

I also made changes to the fromJSON method. This method used to take JSON and return a ChatCraftProvider object. Now it returns either a OpenAiProvider or OpenRouter object depending on the provider url in the json. To do this I had to use dynamic imports.

Code Changes after Review

I have received a code reviews from Taras and Amnish who have both given me advice on how to refactor my code further.

Here are my code changes after receiving their feedback and making the requested changes:

What's Next

My PR is yet to be merged, but once it is, the next step is for me to create another subtask/issue so that we can start using these methods from their provider-specific classes rather than from ai.ts. This will also involve storing our provider settings information into a ChatCraftProvider object in settings.currentProvider or something, so that we can use currentProvider isinstanceof OpenAiProvider and currentProvider isinstanceof OpenRouterProvider to check which provider we are using so that we can call on the correct methods.

My Reviews on Team's PR

Minimize duplication for sidebar code #463

Amnish04 posted on Feb 16, 2024

#437 introduced a lot of duplication for sidebar related styles and logic as we now had different sidebar views for mobile and desktop.

I have tried to minimize that duplication, by creating separate components for mobile and desktop sidebar and exposing through a common Sidebar api, taking inspiration from the way PromptForm is done.

This way, the places that use the sidebar now look the same as before since all the changes have been abstracted in its own module.

Here's a summary of changes:

1. Renamed the existing Sidebar component to SidebarContent.
2. Added a SidebarMobile component that uses SidebarContent and extends it by adding the search field and ChatCraft brand at the top.
3. Similarly, SidebarDesktop component wraps SidebarContent to apply the new open/close transition effect. This way, we don't have to repeat the keyframe definitions at evey place where sidebar is used.
4. These Sidebar components are exposed by the common Sidebar in index.tsx based on the screen width using isMobile hook.

This fixes #457.

View on GitHub

I reviewed Amnish's PR which was about refactoring code to minimize code duplication introduced from his previous sidebar UI changes. I looked through his code, it involved renaming some things and extracting a context for clarity. I also did regression testing on the sidebar functionalities in desktop and mobile to make sure his code did not introduce any issues.