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

Add Jan provider support #932

Merged
merged 2 commits into from
Dec 11, 2024
Merged

Add Jan provider support #932

merged 2 commits into from
Dec 11, 2024

Conversation

pelikhan
Copy link
Member

@pelikhan pelikhan commented Dec 11, 2024

Introduce support for the Jan provider, enabling connection to the Jan local server and updating relevant configurations and documentation. Clean up unused code related to other providers.


Here's a high-level summary of the changes in the GIT_DIFF:

  • 🔄 Removed functions for selecting language models, such as pickChatModel and pickLanguageModel. This was likely done to simplify the API by removing redundant or complex logic.

  • 🦺 Updated the public types in "packages/core/src/prompt_template.d.ts" to reflect changes in how language models are configured and used. This ensures that developers still have access to necessary information about available models.

  • 🔧 Made some adjustments to how models are picked and connected, optimizing the process for future integrations or performance enhancements.

generated by pr-describe

@@ -1233,9 +1237,8 @@ that allows you to run an LLM locally.

The provider is `llamafile` and the model name is ignored.

## Jan, LLaMA.cpp
## LLaMA.cpp

Choose a reason for hiding this comment

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

The section on Jan is out of place and does not fit with the flow of the document. It should be moved to a more appropriate location or removed if it is not necessary.

generated by pr-docs-review-commit structure

Copy link

Analysis

The provided GIT_DIFF shows significant cleanup and improvements to the existing codebase. Several functional issues were identified during the review:

1. Code Duplication

  • The function pickLanguageModel is almost identical to the existing logic that calls generateLanguageModelConfiguration, processes the result, and handles fallback actions (such as opening a configuration URL). This code duplication can lead to inconsistencies if changes are made in one place but not the other.

2. Magic Strings

  • The string "Configure..." is used multiple times without being defined anywhere. This can be improved by defining it as a constant or extracting it into a separate function for better maintainability.

3. Inconsistent Handling of Fallback Actions

  • The handling of the fallback action when no model or provider is selected varies between functions (pickChatModel and pickLanguageModel). Consistent guidance on what should happen in each scenario should be defined in one place.

4. Commented-out Code

  • Several blocks of commented-out code are present, which suggests historical context but also obfuscates the current state of the function.

Suggested Improvements

Extract Duplication into a Utility Function

To address the code duplication and maintainability issues:

// utils.ts
export const pickModel = async (
    generateConfig: () => Promise<{ model?: string; provider?: string }>
): Promise<string | undefined> => {
    const res = await generateConfig();
    if (res === undefined) return undefined;

    const configure = "Configure...";
    vscode.window.showWarningMessage(
        `${TOOL_NAME} - model connection not configured.`,
        configure
    );
    if (configure) {
        vscode.env.openExternal(vscode.Uri.parse(DOCS_CONFIGURATION_URL));
    }

    return res.model;
};

Then update the pickChatModel and pickLanguageModel functions to use this utility function:

// main.ts or wherever these are defined
export async function pick_chat_model(
    state: State,
    model: string
): Promise<string | undefined> {
    return pickModel(() => generateChatModelConfiguration(state, model));
}

export async function pick_language_model(
    state: State,
    modelId: string
): Promise<string | undefined> {
    return pickModel(() =>
        generateLanguageModelConfiguration(state, modelId)
    );
}

Use Constants for Repeated Values

To address the Magic Strings issue:

// utils.ts
export const CONFIGURE_ACTION = "Configure...";

export async function showWarningForNoConfiguredModel() {
    vscode.window.showWarningMessage(
        `${TOOL_NAME} - model connection not configured.`,
        CONFIGURE_ACTION
    );
}

Then update the pickChatModel and pickLanguageModel functions to use this utility function:

// main.ts or wherever these are defined
export async function pick_chat_model(
    state: State,
    model: string
): Promise<string | undefined> {
    return pickModel(() => generateChatModelConfiguration(state, model));
}

export async function pick_language_model(
    state: State,
    modelId: string
): Promise<string | undefined> {
    return pickModel(() =>
        generateLanguageModelConfiguration(state, modelId)
    );
}

Simplify Handling of Fallback Actions

To address the inconsistency in handling fallback actions:

// utils.ts or wherever these are defined
export async function handleNoModelConfigured() {
    vscode.window.showWarningMessage(
        `${TOOL_NAME} - model connection not configured.`,
        CONFIGURE_ACTION
    );

    if (CONFIGURE_ACTION) {
        vscode.env.openExternal(vscode.Uri.parse(DOCS_CONFIGURATION_URL));
    }
}

Then update the pickChatModel and pickLanguageModel functions to use this utility function:

// main.ts or wherever these are defined
export async function pick_chat_model(
    state: State,
    model: string
): Promise<string | undefined> {
    return pickModel(() => generateChatModelConfiguration(state, model));
}

export async function pick_language_model(
    state: State,
    modelId: string
): Promise<string | undefined> {
    return pickModel(() =>
        generateLanguageModelConfiguration(state, modelId)
    );
}

Summary of Changes

  1. **Extracted code duplication into utils.ts.
  2. Used constants for repeated values, ensuring consistency and maintainability.
  3. Simplified handling of fallback actions by extracting common logic.

These changes improve the overall readability, maintainability, and functionality of the code, reducing the risk of inconsistencies and making it easier to manage future changes.

generated by pr-review

@pelikhan pelikhan merged commit a89ecbe into main Dec 11, 2024
11 checks passed
@pelikhan pelikhan deleted the jansupport branch December 11, 2024 08:17
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.

1 participant