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 24 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
14,314 changes: 12,509 additions & 1,805 deletions agent/recordings/defaultClient_631904893/recording.har.yaml

Large diffs are not rendered by default.

131 changes: 83 additions & 48 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 @@ -618,7 +618,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
154 changes: 71 additions & 83 deletions agent/src/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -929,23 +929,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 @@ -961,61 +965,57 @@ 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

No additional packages or imports are needed.
" Okay, based on the shared context, it looks like Vitest is being used as the test framework. No mocks are detected.

I will focus on testing the Animal interface by validating:
Here are some new unit tests for the Animal interface in src/animal.ts using Vitest:
abeatrix marked this conversation as resolved.
Show resolved Hide resolved

- The name property is a string
- makeAnimalSound returns a string
- isMammal is a boolean
\`\`\`ts
import {describe, expect, it} from 'vitest'
import {Animal} from './animal'

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

describe('Animal', () => {

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

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

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

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

it('isMammal is a boolean', () => {
const animal: Animal = {
name: 'Lion',
makeAnimalSound: () => '',
isMammal: true
}
it('has an isMammal property', () => {
const animal: Animal = {
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 @@ -1031,7 +1031,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 @@ -1041,60 +1041,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
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.

69 changes: 69 additions & 0 deletions lib/shared/src/commands/types.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
// The default Cody Commands
export type DefaultCodyCommands = DefaultChatCommands | DefaultEditCommands

// Default Cody Commands that runs as a Chat request
export enum DefaultChatCommands {
Ask = 'chat', // Submit a question in chat
Explain = 'explain', // Explain code
Test = 'test', // Generate unit tests in Chat
Smell = 'smell', // Generate code smell report in Chat
}

// Default Cody Commands that runs as an Inline Edit command
export enum DefaultEditCommands {
Edit = 'edit', // Inline edit request
Unit = 'unit', // Generate unit tests with inline edit
Doc = 'doc', // Generate documentation with inline edit
}

// The blueprint of a Cody Custom Command
export interface CodyCommand {
slashCommand: string
prompt: string
description?: string
context?: CodyCommandContext
type?: CodyCommandType
mode?: CodyCommandMode

// Internal use - the ID of the request
requestID?: string
}

/**
* - 'ask' mode is the default mode, run prompt in chat view
* - 'edit' mode will run prompt with edit command which replace selection with cody's response
* - 'insert' mode is the same as edit, it adds to the top of the selection instead of replacing selection
* - 'file' mode create a new file with cody's response as content
*/
type CodyCommandMode = 'ask' | 'edit' | 'insert' | 'file'

// Type of context available for prompt building
export interface CodyCommandContext {
// Exclude any context.
// It takes precedence over all other context.
none?: boolean

// Tabs from the current workspace
openTabs?: boolean
// The directory with the current file opened in the editor
currentDir?: boolean
// The current file opened in the editor
currentFile?: boolean
// The current selection in the editor
// Default to use smart selection unless set to false specifically
selection?: boolean
// Shell command to run to get context
command?: string
// The relative path of a file within your workspace root
filePath?: string
// The relative path of a directory within your workspace root
directoryPath?: string

// NOTE: Currently not supported
// Codebase context from current codebase
codebase?: boolean
}

export type CodyCommandType = CustomCommandType | 'default' | 'recently used' | 'experimental'

export type CustomCommandType = 'workspace' | 'user'
Loading
Loading