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

Use ConfigResult everywhere #3525

Merged
merged 4 commits into from
Dec 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 12 additions & 12 deletions binary/test/binary.test.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { SerializedContinueConfig } from "core";
// import Mock from "core/llm/llms/Mock.js";
import { FromIdeProtocol, ToIdeProtocol } from "core/protocol/index.js";
import FileSystemIde from "core/util/filesystem";
import { IMessenger } from "core/protocol/messenger";
import FileSystemIde from "core/util/filesystem";
import fs from "fs";
import {
ChildProcessWithoutNullStreams,
Expand Down Expand Up @@ -186,10 +186,11 @@ describe("Test Suite", () => {
});

it("should return valid config object", async () => {
const { config } = await messenger.request(
const { result } = await messenger.request(
"config/getSerializedProfileInfo",
undefined,
);
const { config } = result;
expect(config).toHaveProperty("models");
expect(config).toHaveProperty("embeddingsProvider");
expect(config).toHaveProperty("contextProviders");
Expand Down Expand Up @@ -229,18 +230,17 @@ describe("Test Suite", () => {
await messenger.request("config/addModel", {
model,
});
const { config } = await messenger.request(
"config/getSerializedProfileInfo",
undefined,
);
expect(config.models.some((m) => m.title === model.title)).toBe(true);
const {
result: { config },
} = await messenger.request("config/getSerializedProfileInfo", undefined);

expect(config!.models.some((m) => m.title === model.title)).toBe(true);

await messenger.request("config/deleteModel", { title: model.title });
const { config: configAfterDelete } = await messenger.request(
"config/getSerializedProfileInfo",
undefined,
);
expect(configAfterDelete.models.some((m) => m.title === model.title)).toBe(
const {
result: { config: configAfterDelete },
} = await messenger.request("config/getSerializedProfileInfo", undefined);
expect(configAfterDelete!.models.some((m) => m.title === model.title)).toBe(
false,
);
});
Expand Down
6 changes: 2 additions & 4 deletions core/autocomplete/CompletionProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,9 @@
import { IDE, ILLM } from "../index.js";
import OpenAI from "../llm/llms/OpenAI.js";
import { DEFAULT_AUTOCOMPLETE_OPTS } from "../util/parameters.js";
import { PosthogFeatureFlag, Telemetry } from "../util/posthog.js";

import { shouldCompleteMultiline } from "./classification/shouldCompleteMultiline.js";
import { ContextRetrievalService } from "./context/ContextRetrievalService.js";

Check warning on line 8 in core/autocomplete/CompletionProvider.ts

View workflow job for this annotation

GitHub Actions / core-checks

There should be no empty line within import group
// @prettier-ignore

import { BracketMatchingService } from "./filtering/BracketMatchingService.js";
import { CompletionStreamer } from "./generation/CompletionStreamer.js";
Expand Down Expand Up @@ -123,10 +121,10 @@
}

private async _getAutocompleteOptions() {
const config = await this.configHandler.loadConfig();
const { config } = await this.configHandler.loadConfig();
const options = {
...DEFAULT_AUTOCOMPLETE_OPTS,
...config.tabAutocompleteOptions,
...config?.tabAutocompleteOptions,
};
return options;
}
Expand Down Expand Up @@ -259,7 +257,7 @@

// Save to cache
if (!outcome.cacheHit && helper.options.useCache) {
(await this.autocompleteCache).put(outcome.prefix, outcome.completion);

Check warning on line 260 in core/autocomplete/CompletionProvider.ts

View workflow job for this annotation

GitHub Actions / core-checks

Promises must be awaited, end with a call to .catch, end with a call to .then with a rejection handler or be explicitly marked as ignored with the `void` operator
}

// When using the JetBrains extension, Mark as displayed
Expand Down
4 changes: 2 additions & 2 deletions core/config/ConfigHandler.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ describe.skip("Test the ConfigHandler and E2E config loading", () => {
});

test("should load the default config successfully", async () => {
const config = await testConfigHandler.loadConfig();
expect(config.models.length).toBe(defaultConfig.models.length);
const result = await testConfigHandler.loadConfig();
expect(result.config!.models.length).toBe(defaultConfig.models.length);
});

test.skip("should add a system message from config.ts", async () => {
Expand Down
32 changes: 14 additions & 18 deletions core/config/ConfigHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,13 +60,13 @@ export class ConfigHandler {

// Always load local profile immediately in case control plane doesn't load
try {
this.loadConfig();
void this.loadConfig();
} catch (e) {
console.error("Failed to load config: ", e);
}

// Load control plane profiles
this.fetchControlPlaneProfiles();
void this.fetchControlPlaneProfiles();
}

// This will be the local profile
Expand Down Expand Up @@ -146,12 +146,8 @@ export class ConfigHandler {

async setSelectedProfile(profileId: string) {
this.selectedProfileId = profileId;
const newConfig = await this.loadConfig();
this.notifyConfigListeners({
config: newConfig,
errors: undefined,
configLoadInterrupted: false,
});
const result = await this.loadConfig();
this.notifyConfigListeners(result);
const selectedProfiles =
this.globalContext.get("lastSelectedProfileForWorkspace") ?? {};
selectedProfiles[await this.getWorkspaceId()] = profileId;
Expand All @@ -170,7 +166,7 @@ export class ConfigHandler {
// Automatically refresh config when Continue-related IDE (e.g. VS Code) settings are changed
updateIdeSettings(ideSettings: IdeSettings) {
this.ideSettingsPromise = Promise.resolve(ideSettings);
this.reloadConfig();
void this.reloadConfig();
}

updateControlPlaneSessionInfo(
Expand Down Expand Up @@ -236,24 +232,24 @@ export class ConfigHandler {
return this.profiles.map((p) => p.profileDescription);
}

async loadConfig(): Promise<ContinueConfig> {
return (
await this.currentProfile.loadConfig(this.additionalContextProviders)
).config!; // <-- TODO
async loadConfig(): Promise<ConfigResult<ContinueConfig>> {
return await this.currentProfile.loadConfig(
this.additionalContextProviders,
);
}

async llmFromTitle(title?: string): Promise<ILLM> {
const config = await this.loadConfig();
const model = config.models.find((m) => m.title === title);
const { config } = await this.loadConfig();
const model = config?.models.find((m) => m.title === title);
if (!model) {
if (title === ONBOARDING_LOCAL_MODEL_TITLE) {
// Special case, make calls to Ollama before we have it in the config
const ollama = new Ollama({
model: LOCAL_ONBOARDING_CHAT_MODEL,
});
return ollama;
} else if (config.models.length > 0) {
return config.models[0];
} else if (config?.models?.length) {
return config?.models[0];
}

throw new Error("No model found");
Expand All @@ -264,6 +260,6 @@ export class ConfigHandler {

registerCustomContextProvider(contextProvider: IContextProvider) {
this.additionalContextProviders.push(contextProvider);
this.reloadConfig();
void this.reloadConfig();
}
}
37 changes: 19 additions & 18 deletions core/config/ProfileLifecycleManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ export interface ProfileDescription {
}

export class ProfileLifecycleManager {
private savedConfig: ConfigResult<ContinueConfig> | undefined;
private savedBrowserConfig?: BrowserSerializedContinueConfig;
private savedConfigResult: ConfigResult<ContinueConfig> | undefined;
private savedBrowserConfigResult?: ConfigResult<BrowserSerializedContinueConfig>;
private pendingConfigPromise?: Promise<ConfigResult<ContinueConfig>>;

constructor(private readonly profileLoader: IProfileLoader) {}
Expand All @@ -35,15 +35,15 @@ export class ProfileLifecycleManager {
}

clearConfig() {
this.savedConfig = undefined;
this.savedBrowserConfig = undefined;
this.savedConfigResult = undefined;
this.savedBrowserConfigResult = undefined;
this.pendingConfigPromise = undefined;
}

// Clear saved config and reload
async reloadConfig(): Promise<ConfigResult<ContinueConfig>> {
this.savedConfig = undefined;
this.savedBrowserConfig = undefined;
this.savedConfigResult = undefined;
this.savedBrowserConfigResult = undefined;
this.pendingConfigPromise = undefined;

return this.loadConfig([], true);
Expand All @@ -55,8 +55,8 @@ export class ProfileLifecycleManager {
): Promise<ConfigResult<ContinueConfig>> {
// If we already have a config, return it
if (!forceReload) {
if (this.savedConfig) {
return this.savedConfig;
if (this.savedConfigResult) {
return this.savedConfigResult;
} else if (this.pendingConfigPromise) {
return this.pendingConfigPromise;
}
Expand All @@ -72,7 +72,7 @@ export class ProfileLifecycleManager {
result.config.contextProviders ?? []
).concat(additionalContextProviders);

this.savedConfig = result;
this.savedConfigResult = result;
resolve(result);
} else if (result.errors) {
reject(
Expand All @@ -82,28 +82,29 @@ export class ProfileLifecycleManager {
});

// Wait for the config promise to resolve
this.savedConfig = await this.pendingConfigPromise;
this.savedConfigResult = await this.pendingConfigPromise;
this.pendingConfigPromise = undefined;
return this.savedConfig;
return this.savedConfigResult;
}

async getSerializedConfig(
additionalContextProviders: IContextProvider[],
): Promise<ConfigResult<BrowserSerializedContinueConfig>> {
if (!this.savedBrowserConfig) {
if (this.savedBrowserConfigResult) {
return this.savedBrowserConfigResult;
} else {
const result = await this.loadConfig(additionalContextProviders);
if (!result.config) {
return {
...result,
config: undefined,
};
}
this.savedBrowserConfig = finalToBrowserConfig(result.config);
const serializedConfig = finalToBrowserConfig(result.config);
return {
...result,
config: serializedConfig,
};
}
return {
config: this.savedBrowserConfig,
errors: [],
configLoadInterrupted: false,
};
}
}
5 changes: 3 additions & 2 deletions core/context/mcp/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { Transport } from "@modelcontextprotocol/sdk/shared/transport.js";
import { ContinueConfig, MCPOptions, SlashCommand, Tool } from "../..";
import { constructMcpSlashCommand } from "../../commands/slash/mcp";
import { ConfigValidationError } from "../../config/validation";
import { encodeMCPToolUri } from "../../tools/callTool";
import MCPContextProvider from "../providers/MCPContextProvider";

export class MCPManagerSingleton {
Expand Down Expand Up @@ -132,7 +133,7 @@ class MCPConnection {
if (!error.message.startsWith("StdioClientTransport already started")) {
return {
fatal: false,
message: `Failed to connect to MCP ${mcpId}: ${error.message}`,
message: `Failed to connect to MCP: ${error.message}`,
};
}
}
Expand Down Expand Up @@ -172,7 +173,7 @@ class MCPConnection {
readonly: false,
type: "function",
wouldLikeTo: `use the ${tool.name} tool`,
uri: `mcp://${tool.name}`,
uri: encodeMCPToolUri(mcpId, tool.name),
}));

config.tools = [...config.tools, ...continueTools];
Expand Down
9 changes: 6 additions & 3 deletions core/context/providers/_context-providers.test.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
import fetch from "node-fetch";

import { contextProviderClassFromName } from ".";
import {
ContextProviderExtras,
ContextProviderWithParams,
IContextProvider,
} from "../..";
import { ConfigHandler } from "../../config/ConfigHandler";
import { contextProviderClassFromName } from ".";
import { ControlPlaneClient } from "../../control-plane/client";
import FileSystemIde from "../../util/filesystem";
import { TEST_DIR } from "../../test/testDir";
import FileSystemIde from "../../util/filesystem";

const CONTEXT_PROVIDERS_TO_TEST: ContextProviderWithParams[] = [
{ name: "diff", params: {} },
Expand All @@ -33,7 +33,10 @@ async function getContextProviderExtras(
async (text) => {},
new ControlPlaneClient(Promise.resolve(undefined)),
);
const config = await configHandler.loadConfig();
const { config } = await configHandler.loadConfig();
if (!config) {
throw new Error("Config not found");
}

return {
fullInput,
Expand Down
Loading
Loading