Skip to content

Commit

Permalink
Decompose ChatController.sendChat into handlers for different request…
Browse files Browse the repository at this point in the history
… types (#6469)

Factor out the main user message submission handler
(`ChatController.sendChat`) into multiple handlers:
- `ChatHandler`
- `DeepCodyHandler`
- `SearchHandler`

Previously, `ChatController.sendChat` was a long long method that
implicitly encoded these different types of response handlers through
various conditionals and difficult-to-reason-about state. Now, each
request types gets its own handler class, which more clearly separates
concerns. This will allow us to implement more complex logic for each
request type without breaking encapsulation, and it also opens the door
to more easily add new types of agentic handlers in the future.

Also remove the `edit` and `insert` message intents, which seem to be
defunct/unused.

There should be no behavior change. Telemetry behavior is preserved.
  • Loading branch information
beyang authored Dec 27, 2024
1 parent 30145b9 commit f515283
Show file tree
Hide file tree
Showing 16 changed files with 2,021 additions and 1,907 deletions.
1,018 changes: 487 additions & 531 deletions agent/recordings/customCommandsClient_509552979/recording.har.yaml

Large diffs are not rendered by default.

1,146 changes: 522 additions & 624 deletions agent/recordings/defaultClient_631904893/recording.har.yaml

Large diffs are not rendered by default.

3 changes: 1 addition & 2 deletions agent/src/__snapshots__/custom-commands.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

exports[`Custom Commands > commands/custom, chat command, open tabs context 1`] = `
"Here are all the files you've shared:
• src/example1.ts
• src/example2.ts
• src/example3.ts"
Expand All @@ -13,7 +12,7 @@ exports[`Custom Commands > commands/custom, edit command, edit mode 1`] = `
name: string
makeAnimalSound(): string
isMammal: boolean
logName(): void
logAnimalName(): void
}"
`;

Expand Down
162 changes: 76 additions & 86 deletions agent/src/__snapshots__/index.test.ts.snap
Original file line number Diff line number Diff line change
@@ -1,44 +1,46 @@
// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html

exports[`Agent > Chat > chat/submitMessage (long message) 1`] = `
"I'll create a simple Hello World function in Java!
"I'll create a simple Hello World function in Java for you.
Here's a clear and standard Hello World implementation:
\`\`\`java:HelloWorld.java
\`\`\`java:src/HelloWorld.java
public class HelloWorld {
public static void main(String[] args) {
sayHello();
}
public static void sayHello() {
System.out.println("Hello, World!");
}
}
\`\`\`
To compile and run this Java program, use these commands:
To compile and run this Java program, you can use these commands:
\`\`\`bash
javac HelloWorld.java
javac src/HelloWorld.java
\`\`\`
\`\`\`bash
java HelloWorld
java -cp src HelloWorld
\`\`\`
This code demonstrates the basic structure of a Java program with a main method that prints "Hello, World!" to the console. The code is straightforward and follows Java conventions perfectly."
This creates a clean and simple Hello World program with a dedicated function to print the greeting. The program uses a separate \`sayHello()\` method which demonstrates good practice for function organization. Feel free to modify the message or add more functionality!"
`;

exports[`Agent > Chat > chat/submitMessage (short message) 1`] = `
{
"model": "anthropic::2024-10-22::claude-3-5-sonnet-latest",
"speaker": "assistant",
"text": "Hi there! Great to meet you! I'm Cody, ready to help you with any coding or technical questions you have. What would you like to work on?",
"text": "Hi there! Great to meet you! I'm Cody, and I'm excited to help you with any coding or technical questions you may have. What would you like to work on?",
}
`;

exports[`Agent > Chat > chat/submitMessage (with mock context) 1`] = `
"\`\`\`typescript:src/dog.ts
export class Dog implements Animal {
name: string;
isMammal: boolean = true;
isMammal = true;
constructor(name: string) {
this.name = name;
Expand All @@ -52,95 +54,83 @@ export class Dog implements Animal {
`;

exports[`Agent > Commands > commands/explain 1`] = `
"The code you've shared is an excerpt from a TypeScript file, specifically an interface named \`Animal\`.
The purpose of this code is to define a blueprint for an object that represents an animal. An interface in TypeScript works as a contract that defines the structure and behavior that an object implementing this interface should have.
Let's break down the code to understand what it does:
1. \`export interface Animal\`: This line declares an interface named \`Animal\`. By prefixing \`export\`, it allows other files or modules to import and use this interface.
2. \`name: string\`: This line defines the first property in the \`Animal\` interface, which is \`name\`. This property's type should be of type \`string\`.
3. \`makeAnimalSound(): string\`: This line introduces a method called \`makeAnimalSound\`. It does not take any input parameters, and it returns a value of type \`string\`. The intention of this method is to represent the sound that the animal makes.
4. \`isMammal: boolean\`: This line defines a property named \`isMammal\` with type \`boolean\`. This indicates whether the animal is a mammal or not.
"The code being explained is an interface for an object called \`Animal\` in the file \`animal.ts\`. This code defines the shape that an Animal object must take in order to be considered an Animal in the codebase.
In summary, the \`Animal\` interface serves as a simple blueprint for what an object representing an animal should contain and look like. It expects the implementing object to have the following:
1. The purpose of the code: The code is defining an \`Animal\` interface. This means that if any object is to be considered an \`Animal\`, it must have at least the following properties: \`name\` (a string), \`makeAnimalSound\` (a method that returns a string), and \`isMammal\` (a boolean value).
2. Inputs: This code, as an interface, doesn't have inputs in the traditional sense. However, any object that wants to conform to the shape of an \`Animal\` will need to provide values for the required properties and methods defined in the \`Animal\` interface. For example, another code file might create an object like so: \`let myDog: Animal = { name: 'Fido', makeAnimalSound: () => 'Woof!', isMammal: true }\`.
3. Outputs: The code doesn't produce outputs as it is just an interface. However, it enables other code to define objects with a consistent structure.
4. Algorithm: The code uses TypeScript syntax to define an interface. This interface consists of three properties (\`name\`, \`makeAnimalSound\`, \`isMammal\`), each with a specific type; \`string\`, \`() => string\`, and \`boolean\`, respectively.
5. Logic and data transformations: Since this is an interface, there are no actual logic and data transformations happening. However, it provides a blueprint for other objects so that they can implement the required logic and data transformations.
* A \`name\` property as a \`string\`.
* A \`makeAnimalSound\` method that returns a \`string\`.
* An \`isMammal\` property as a \`boolean\`.
This blueprint does not define input(s) as it only serves as a contract or a structure, and it produces different outputs depending on what kind of animal object adheres to the contract. The interface helps us ensure that provided animal objects contain the right data and behavior, which makes it easier to work with such objects throughout the codebase.
A beginner should note that TypeScript interfaces help define data structures and behaviors for easier object manipulation, ensuring better consistency during coding while catching potential errors early on."
In summary, the given code describes the Animal interface, providing a blueprint for other objects to implement the required shape and behavior. Working with this blueprint, programmers can maintain consistency throughout the codebase and implement specific functionalities according to their demands."
`;

exports[`Agent > Commands > commands/smell 1`] = `
"Here are my suggestions:
1. **Add type annotations to the methods of the \`Animal\` interface:**
Adding type annotations to the \`makeAnimalSound\` method, like this:
\`\`\`typescript
makeAnimalSound(): string
\`\`\`
will ensure that the method returns a string, and will make it clear to other developers what type of value the method should return.
2. **Add a constructor function to the \`Animal\` interface:**
Adding a constructor function to the \`Animal\` interface, like this:
\`\`\`typescript
construct(name: string, isMammal: boolean)
\`\`\`
will make it clear how an \`Animal\` object should be initialized, and will ensure that all \`Animal\` objects have the necessary properties.
3. **Use \`readonly\` properties for the \`name\` and \`isMammal\` properties:**
Using \`readonly\` properties for the \`name\` and \`isMammal\` properties, like this:
"Based on the provided code from \`@src/animal.ts:1-6\`:
\`\`\`typescript
export interface Animal {
readonly name: string
readonly isMammal: boolean
name: string
makeAnimalSound(): string
isMammal: boolean
}
\`\`\`
will ensure that the \`name\` and \`isMammal\` properties cannot be changed after an \`Animal\` object is created. This can help prevent bugs caused by accidental changes to these properties.
4. **Use a \`class\` instead of an \`interface\` to define \`Animal\`:**
Using a \`class\` instead of an \`interface\` to define \`Animal\`, like this:
\`\`\`typescript
export class Animal {
constructor(public name: string, public isMammal: boolean) { }
makeAnimalSound(): string {
throw new Error("Method 'makeAnimalSound' must be implemented.");
Here are 5 suggestions to improve the code:
1. **Add type annotations for the methods.** Adding type annotations for the methods in the interface could improve type safety and self-document the code.
\`\`\`typescript
export interface Animal {
name: string
makeAnimalSound(): string
isMammal: boolean
eat(food: string): void
}
\`\`\`
Benefit: Enhanced type safety, and clearer understanding of intended usage.
2. **Add access modifiers.** By specifying access modifiers (public, private, protected) for members, we can ensure encapsulation and control access.
\`\`\`typescript
export interface Animal {
name: string
makeAnimalSound(): string
readonly isMammal: boolean // readonly ensures that this property cannot be changed
}
\`\`\`
Benefit: Improved encapsulation, and consistency in codebase.
3. **Follow PascalCase for naming conventions in TypeScript.** Use PascalCase for method names inside the interface.
\`\`\`typescript
export interface Animal {
name: string
MakeAnimalSound(): string
isMammal: boolean
}
\`\`\`
Benefit: Adherence to community conventions and improved readability.
4. **Use enums when defining a set of named values.** Instead of using boolean values for \`isMammal\`, use an enum.
\`\`\`typescript
export enum AnimalType {
Mammal,
Reptile,
Amphibian
}
}
\`\`\`
will allow you to define a constructor and common methods that all \`Animal\` objects will inherit.
5. **Add comments to the code:**
Adding comments to the code, like this:
\`\`\`typescript
/**
* An interface for defining an animal.
*/
export interface Animal {
/** The name of the animal. */
name: string
export interface Animal {
name: string
makeAnimalSound(): string
type: AnimalType
}
\`\`\`
Benefit: Provides a clearer, more self-descriptive representation of the data.
5. **Add JSDoc comments for TypeScript interfaces.** JSDoc comments make code more readable and provide useful information when interacting with code through an IDE.
\`\`\`typescript
/**
* Makes the animal sound.
* @returns The sound that the animal makes.
* Animal interface
*/
makeAnimalSound(): string
/** Whether the animal is a mammal. */
isMammal: boolean
}
\`\`\`
will make the code easier to understand for other developers, and will help them use the \`Animal\` interface correctly.
export interface Animal {
name: string
makeAnimalSound(): string
isMammal: boolean
}
\`\`\`
Benefit: Improved readability and discoverability of interface usage in IDEs.
Overall, the code is well-written and follows good design principles. However, adding type annotations, constructor functions, \`readonly\` properties, a \`class\` definition, and comments could make it even more robust, efficient, and easy to understand."
Overall, the provided code snippet demonstrates a good foundation for a TypeScript interface. The main opportunities for improvement center around adhering to TypeScript conventions and enhancing code readability for both humans and integrated development environments."
`;
2 changes: 1 addition & 1 deletion agent/src/custom-commands.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ describe('Custom Commands', () => {
expect(result.type).toBe('chat')
const lastMessage = await client.firstNonEmptyTranscript(result.chatResult as string)
const reply = trimEndOfLine(lastMessage.messages.at(-1)?.text ?? '')
expect(reply).toMatchInlineSnapshot(`"7"`, explainPollyError)
expect(reply).toMatchInlineSnapshot(`"6"`, explainPollyError)
}, 30_000)

it('commands/custom, edit command, insert mode', async () => {
Expand Down
4 changes: 2 additions & 2 deletions agent/src/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -197,8 +197,8 @@ describe('Agent', () => {
expect(currentUserCodySubscription).toMatchInlineSnapshot(`
{
"applyProRateLimits": true,
"currentPeriodEndAt": "2024-12-14T22:11:32Z",
"currentPeriodStartAt": "2024-11-14T22:11:32Z",
"currentPeriodEndAt": "2025-01-14T22:11:32Z",
"currentPeriodStartAt": "2024-12-14T22:11:32Z",
"plan": "PRO",
"status": "ACTIVE",
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,9 @@ class CodyFixHighlightPass(val file: PsiFile, val editor: Editor) :
@RequiresEdt
override fun doApplyInformationToEditor() {
for (highlight in myHighlights) {
highlight.unregisterQuickFix { (it as? CodeActionQuickFix)?.familyName == CodeActionQuickFix.FAMILY_NAME }
highlight.unregisterQuickFix {
(it as? CodeActionQuickFix)?.familyName == CodeActionQuickFix.FAMILY_NAME
}

if (highlight.startOffset > document.textLength ||
highlight.endOffset > document.textLength ||
Expand Down
12 changes: 6 additions & 6 deletions vscode/src/chat/chat-view/ChatController.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ describe('ChatController', () => {
signal: new AbortController().signal,
source: 'chat',
})
expect(postMessageSpy.mock.calls.at(5)?.at(0)).toStrictEqual<
expect(postMessageSpy.mock.calls.at(4)?.at(0)).toStrictEqual<
Extract<ExtensionMessage, { type: 'transcript' }>
>({
type: 'transcript',
Expand All @@ -146,7 +146,7 @@ describe('ChatController', () => {
search: undefined,
error: undefined,
editorState: null,
contextFiles: [],
contextFiles: undefined,
processes: undefined,
},
{
Expand All @@ -167,7 +167,7 @@ describe('ChatController', () => {
await vi.runOnlyPendingTimersAsync()
expect(mockChatClient.chat).toBeCalledTimes(1)
expect(addBotMessageSpy).toHaveBeenCalledWith('1', ps`Test reply 1`, 'my-model')
expect(postMessageSpy.mock.calls.at(6)?.at(0)).toStrictEqual<
expect(postMessageSpy.mock.calls.at(5)?.at(0)).toStrictEqual<
Extract<ExtensionMessage, { type: 'transcript' }>
>({
type: 'transcript',
Expand Down Expand Up @@ -381,7 +381,7 @@ describe('ChatController', () => {
await vi.runOnlyPendingTimersAsync()
expect(mockChatClient.chat).toBeCalledTimes(1)
expect(addBotMessageSpy).toHaveBeenCalledWith('1', ps`Test partial reply`, 'my-model')
expect(postMessageSpy.mock.calls.at(7)?.at(0)).toStrictEqual<
expect(postMessageSpy.mock.calls.at(8)?.at(0)).toStrictEqual<
Extract<ExtensionMessage, { type: 'transcript' }>
>({
type: 'transcript',
Expand All @@ -402,12 +402,12 @@ describe('ChatController', () => {
},
{
speaker: 'assistant',
model: undefined,
model: FIXTURE_MODEL.id,
error: errorToChatError(new Error('my-error')),
intent: undefined,
manuallySelectedIntent: undefined,
editorState: undefined,
text: undefined,
text: 'Test partial reply',
contextFiles: undefined,
search: undefined,
processes: undefined,
Expand Down
Loading

0 comments on commit f515283

Please sign in to comment.