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

refactor: commands #2869

Merged
merged 36 commits into from
Jan 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
70a66d5
refactor: default commands
abeatrix Jan 23, 2024
18763e8
wip: clean up commands
abeatrix Jan 23, 2024
e9d0f5f
update agent test
abeatrix Jan 23, 2024
e876f42
wip: clean up menus and providers
abeatrix Jan 24, 2024
a3f215d
wip: context
abeatrix Jan 24, 2024
988e334
wip: clean up command names, singleton controller
abeatrix Jan 24, 2024
17409af
remove getContextForCommand
abeatrix Jan 24, 2024
a8d52a1
update agent test
abeatrix Jan 24, 2024
3dc6170
Merge branch 'main' into bee/refactor-commands
abeatrix Jan 24, 2024
76dab65
wip
abeatrix Jan 25, 2024
c9a2749
wip
abeatrix Jan 25, 2024
936abcc
update dir
abeatrix Jan 25, 2024
281ec2a
add e2e tests
abeatrix Jan 25, 2024
fe8cc26
Add unit tests, stronger type
abeatrix Jan 25, 2024
cd72f81
add e2e tests for custom command context, menu, and builder
abeatrix Jan 25, 2024
2cb7808
add error handling to command context
abeatrix Jan 26, 2024
b53e0c8
remove codebase context from menu
abeatrix Jan 26, 2024
309d05a
clean up menu items
abeatrix Jan 26, 2024
5a3a3e7
Merge branch 'main' into bee/refactor-commands
abeatrix Jan 26, 2024
75ad94f
Merge branch 'main' into bee/refactor-commands
abeatrix Jan 26, 2024
e9b9384
fix e2e
abeatrix Jan 26, 2024
09faa36
fix flanky tests
abeatrix Jan 26, 2024
c1c799f
again
abeatrix Jan 26, 2024
f72b691
mark to do
abeatrix Jan 26, 2024
197c597
Merge branch 'main' into bee/refactor-commands
abeatrix Jan 26, 2024
7efa348
clean up and fix menu, add type strong
abeatrix Jan 26, 2024
01f2367
add new e2e test for custom command menu
abeatrix Jan 26, 2024
104e946
fix e2e tests on windows
abeatrix Jan 26, 2024
81c3f4b
update agent test
abeatrix Jan 26, 2024
33bb570
update and add tests
abeatrix Jan 27, 2024
c8d6717
fix lint
abeatrix Jan 27, 2024
1a2e119
support windows path
abeatrix Jan 27, 2024
c5d7774
ooops
abeatrix Jan 27, 2024
62c923f
it was the .only in test
abeatrix Jan 27, 2024
64bded2
Merge branch 'main' into bee/refactor-commands
abeatrix Jan 29, 2024
a8ef81d
Merge branch 'main' into bee/refactor-commands
abeatrix Jan 29, 2024
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
25,977 changes: 21,517 additions & 4,460 deletions agent/recordings/defaultClient_631904893/recording.har.yaml

Large diffs are not rendered by default.

152 changes: 91 additions & 61 deletions agent/recordings/enterpriseClient_3965582033/recording.har.yaml

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion agent/src/agent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -630,7 +630,7 @@ export class Agent extends MessageHandler {
})

// The arguments to pass to the command to make sure edit commands would also run in chat mode
const commandArgs = [{ runInChatMode: true, source: 'editor' }]
const commandArgs = [{ source: 'editor' }]

this.registerAuthenticatedRequest('commands/explain', () => {
return this.createChatPanel(
Expand Down
141 changes: 62 additions & 79 deletions agent/src/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -495,23 +495,27 @@ describe('Agent', () => {
const lastMessage = await client.firstNonEmptyTranscript(id)
expect(trimEndOfLine(lastMessage.messages.at(-1)?.text ?? '')).toMatchInlineSnapshot(
`
" The Animal interface:
" Here is an explanation of the selected TypeScript code in simple terms:

The Animal interface defines the shape of objects that represent animals. Interfaces in TypeScript are used to define the structure of an object - what properties and methods it should have.
The Animal interface

This interface has three properties:
The Animal interface defines the shape of an Animal object. It does not contain any actual implementation, just declarations for what properties and methods an Animal object should have.

1. name - This will be a string property to represent the animal's name.
The interface has three members:

2. makeAnimalSound() - This is a method that will be implemented by classes that implement the Animal interface. It allows each animal to have its own implementation of making a sound.
1. name - This is a string property to store the animal's name.

3. isMammal - This is a boolean property that will specify if the animal is a mammal or not.
2. makeAnimalSound() - This is a method that should return a string representing the sound the animal makes.

The interface does not contain any actual implementation, just the definition of what properties and methods any class implementing Animal should have. This allows us to define a consistent structure that can be reused across different animal classes.
3. isMammal - This is a boolean property that indicates if the animal is a mammal or not.

By defining an Animal interface, we can then create multiple classes like Dog, Cat, Bird etc that implement the interface and provide their own specific logic while ensuring they match the general Animal structure.
The purpose of this interface is to define a consistent structure for Animal objects. Any class that implements the Animal interface will be required to have these three members with these types.

Interfaces are a powerful way to define contracts in TypeScript code and allow different implementations to guarantee they can work together smoothly. This Animal interface creates a reusable way to model animals in a type-safe way."
This allows code dealing with Animal objects to know it can call animal.makeAnimalSound() and access animal.name and animal.isMammal, regardless of the specific class. The interface defines the contract between the implementation and usage of Animals, without caring about specific details.

Interfaces like this are useful for writing reusable code that can handle different types of objects in a consistent way. The code using the interface doesn't need to know about the specifics of each class, just that it implements the Animal interface. This allows easily extending the code to handle new types of animals by simply creating a new class implementing Animal.

So in summary, the Animal interface defines the input expectations and output guarantees for objects representing animals in the system, allowing code to work with any animal in a generic way based on this contract."
`,
explainPollyError
)
Expand All @@ -529,60 +533,56 @@ describe('Agent', () => {
const lastMessage = await client.firstNonEmptyTranscript(id)
expect(trimEndOfLine(lastMessage.messages.at(-1)?.text ?? '')).toMatchInlineSnapshot(
`
" Okay, based on the shared context, it looks like:

- The test framework in use is Vitest
- The assertion library is vitest's built-in expect
" Okay, based on the shared context, it looks like Vitest is being used as the test framework. No mocks are detected.

No additional packages or imports are needed.

I will focus on testing the Animal interface by validating:

- The name property is a string
- makeAnimalSound returns a string
- isMammal is a boolean
Here are some new unit tests for the Animal interface in src/animal.ts using Vitest:

\`\`\`ts
import { expect } from 'vitest'
import { describe, it } from 'vitest'
import { Animal } from './animal'
import {describe, expect, it} from 'vitest'
import {Animal} from './animal'

describe('Animal', () => {

it('has a name property that is a string', () => {
it('has a name property', () => {
const animal: Animal = {
name: 'Lion',
makeAnimalSound: () => '',
name: 'Leo',
makeAnimalSound() {
return 'Roar'
},
isMammal: true
}

expect(typeof animal.name).toBe('string')
expect(animal.name).toBe('Leo')
})

it('makeAnimalSound returns a string', () => {
it('has a makeAnimalSound method', () => {
const animal: Animal = {
name: 'Lion',
makeAnimalSound: () => 'Roar!',
name: 'Whale',
makeAnimalSound() {
return 'Whistle'
},
isMammal: true
}

expect(typeof animal.makeAnimalSound()).toBe('string')
expect(animal.makeAnimalSound()).toBe('Whistle')
})

it('isMammal is a boolean', () => {
it('has an isMammal property', () => {
const animal: Animal = {
name: 'Lion',
makeAnimalSound: () => '',
isMammal: true
name: 'Snake',
makeAnimalSound() {
return 'Hiss'
},
isMammal: false
}

expect(typeof animal.isMammal).toBe('boolean')
expect(animal.isMammal).toBe(false)
})

})
\`\`\`

This covers basic validation of the Animal interface's properties and methods. Let me know if you'd like me to expand the tests further."
This covers basic validation of the Animal interface properties and methods using Vitest assertions. Additional test cases could be added for more edge cases."
`,
explainPollyError
)
Expand All @@ -599,7 +599,7 @@ describe('Agent', () => {
`
" Here are 5 potential improvements for the selected TypeScript code:

1. Add type annotations for method parameters and return types:
1. Add type annotations for method parameters and return values:

\`\`\`
export interface Animal {
Expand All @@ -609,60 +609,48 @@ describe('Agent', () => {
}
\`\`\`

Adding explicit types for methods makes the interface clearer and allows TypeScript to catch more errors at compile time.
Adding type annotations makes the code more self-documenting and enables stronger type checking.

2. Make name readonly:
2. Make interface name more semantic:

\`\`\`
export interface Animal {
readonly name: string
export interface Creature {
// ...
}
\`\`\`

This prevents the name from being reassigned after initialization, making the code more robust.
The name 'Animal' is not very descriptive. A name like 'Creature' captures the intent better.

3. Add alternate method name:
3. Make sound method name more semantic:

\`\`\`
export interface Animal {

// ...

getSound(): string
}
makeSound()
\`\`\`

Adding a method like \`getSound()\` as an alias for \`makeAnimalSound()\` improves readability.
The name 'makeAnimalSound' is verbose. A shorter name like 'makeSound' conveys the purpose clearly.

4. Use boolean getter instead of property for isMammal:
4. Use boolean type for isMammal property:

\`\`\`
export interface Animal {

// ...

get isMammal(): boolean
}
isMammal: boolean
\`\`\`

This allows encapsulation of the logic for determining if mammal.
Using the boolean type instead of just true/false improves readability.

5. Extend a base interface like LivingThing:
5. Add JSDoc comments for documentation:

\`\`\`
interface LivingThing {
name: string
}

interface Animal extends LivingThing {
/**
* Represents a creature in the game
*/
export interface Creature {
// ...
}
\`\`\`

This improves maintainability by separating common properties into a base interface.
JSDoc comments enable generating API documentation and improve understandability.

Overall, the code is well-written but could benefit from some minor changes like adding types, encapsulation, and semantic method names. The interface follows sound principles like read-only properties andboolean getters. No major issues were found."
Overall, the selected code follows reasonable design principles. The interface encapsulates animal data nicely. The suggestions above would incrementally improve quality but no major issues were found."
`,
explainPollyError
)
Expand Down Expand Up @@ -702,10 +690,7 @@ describe('Agent', () => {
check('commands/document (basic function)', 'sum.ts', obtained =>
expect(obtained).toMatchInlineSnapshot(`
"/**
* Sums two numbers.
* @param a - The first number to sum.
* @param b - The second number to sum.
* @returns The sum of a and b.
* Returns the sum of two numbers.
*/
export function sum(a: number, b: number): number {
/* CURSOR */
Expand All @@ -722,8 +707,7 @@ describe('Agent', () => {
constructor(private shouldGreet: boolean) {}

/**
* Prints "Hello World!" to the console if this.shouldGreet is true.
* This allows conditionally greeting the user.
* Logs a greeting if the shouldGreet property is true.
*/
public functionName() {
if (this.shouldGreet) {
Expand All @@ -739,8 +723,7 @@ describe('Agent', () => {
expect(obtained).toMatchInlineSnapshot(`
"const foo = 42
/**
* TestLogger object that contains a startLogging method to initialize logging.
* startLogging sets up a recordLog function that writes log messages to the console.
* Starts logging by initializing and calling the \`recordLog\` function.
*/
export const TestLogger = {
startLogging: () => {
Expand All @@ -764,12 +747,12 @@ describe('Agent', () => {
import { describe } from 'vitest'

/**
* Test block that runs a set of test cases.
* A test block with multiple test cases.
*
* Contains 3 test cases:
* - 'does 1' checks an expectation
* - 'does 2' checks an expectation
* - 'does something else' has a commented out line that errors
* - 'does 1' asserts that true equals true
* - 'does 2' asserts that true equals true
* - 'does something else' has a commented out line that would error due to incorrect usage of \`performance.now\`
*/
describe('test block', () => {
it('does 1', () => {
Expand Down
4 changes: 2 additions & 2 deletions lib/shared/src/chat/transcript/messages.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import type { ContextFile, PreciseContext } from '../../codebase-context/messages'
import type { CodyDefaultCommands } from '../../commands'
import type { Message } from '../../sourcegraph-api'

import type { TranscriptJSON } from '.'
import type { DefaultCodyCommands } from '../../commands/types'

export interface ChatButton {
label: string
Expand Down Expand Up @@ -68,7 +68,7 @@ export type ChatEventSource =
| 'custom-commands'
| 'test'
| 'code-lens'
| CodyDefaultCommands
| DefaultCodyCommands

/**
* Converts an Error to a ChatError. Note that this cannot be done naively,
Expand Down
49 changes: 0 additions & 49 deletions lib/shared/src/commands/index.ts

This file was deleted.

Loading
Loading