Skip to content

Commit

Permalink
Chat: enable editing chat command prompts (#3243)
Browse files Browse the repository at this point in the history
  • Loading branch information
abeatrix authored Feb 23, 2024
1 parent 1647266 commit b3649f2
Show file tree
Hide file tree
Showing 13 changed files with 5,147 additions and 5,610 deletions.
10,545 changes: 5,059 additions & 5,486 deletions agent/recordings/defaultClient_631904893/recording.har.yaml

Large diffs are not rendered by default.

70 changes: 32 additions & 38 deletions agent/src/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -672,21 +672,21 @@ describe('Agent', () => {
const lastMessage = await client.firstNonEmptyTranscript(id)
expect(trimEndOfLine(lastMessage.messages.at(-1)?.text ?? '')).toMatchInlineSnapshot(
`
" Animal Interface
" The code defines an Animal interface:
The selected code defines an interface called Animal. An interface in TypeScript is like a blueprint or contract that defines the structure of an object. Specifically:
The Animal interface defines the structure of an animal object. It takes no inputs. The purpose is to define the properties and methods that any animal object should have.
1. The purpose of this Animal interface is to define the shape of Animal objects - that is, to specify what properties and methods any object implementing the Animal interface must have.
It has three members:
2. The Animal interface itself does not take any inputs. However, any concrete classes that implement the Animal interface would take inputs to instantiate an object (for example, constructor arguments).
1. name - This is a string property that represents the animal's name. All animals should have a name.
3. Similarly, the Animal interface does not directly produce any outputs. Its role is to define the shape of objects. Any class implementing Animal would be responsible for producing outputs through its concrete methods and properties.
2. makeAnimalSound() - This is a method that returns a string. It represents the sound the animal makes. All animals should be able to make a sound.
4. The interface achieves its purpose by declaring a name property of type string, a makeAnimalSound method that returns a string, and an isMammal property of type boolean.
3. isMammal - This is a boolean property that indicates if the animal is a mammal or not. All animals can be classified as mammals or not mammals.
5. By defining this structure, the interface ensures that any class implementing Animal will have these required members with the specified types. This allows TypeScript to enforce that objects adhere to the Animal contract.
By defining this interface, we can then create animal objects of different types like Dog, Cat, Cow etc that follow this structure. They will be guaranteed to have these members that we can rely on when writing code that interacts with animal objects.
So in summary, the Animal interface defines a blueprint for Animal objects by specifying required properties and methods. This allows TypeScript to statically analyze objects based on their fulfillment of the interface contract. The interface itself does not contain logic - it simply defines the shape of objects. Any consuming code can then rely on objects implementing the Animal interface having the defined structure."
So in summary, it defines a consistent structure for animal data without needing to know the specific animal type. The interface itself doesn't contain implementation details, it just defines the contract. Classes that implement the Animal interface would provide the actual implementation."
`,
explainPollyError
)
Expand Down Expand Up @@ -772,58 +772,52 @@ describe('Agent', () => {
`
" Here are 5 potential improvements for the selected TypeScript code:
1. Add type annotations for method parameters and return values:
1. Add type annotations for method parameters and return types:
\`\`\`
export interface Animal {
name: string
makeAnimalSound(volume?: number): string
isMammal: boolean
name: string;
makeAnimalSound(volume?: number): string;
isMammal: boolean;
}
\`\`\`
Adding type annotations makes the code more self-documenting and enables stronger type checking.
Adding annotations improves understandability and enables stronger type checking.
2. Make interface name more semantic:
2. Make interface properties readonly if they are not meant to be reassigned:
\`\`\`
export interface Creature {
// ...
export interface Animal {
readonly name: string;
makeAnimalSound(volume?: number): string;
readonly isMammal: boolean;
}
\`\`\`
The name 'Animal' is not very descriptive. A name like 'Creature' captures the intent better.
Readonly properties clarify intent and prevent accidental mutation.
3. Make sound method name more semantic:
3. Use more specific parameter names than volume for makeAnimalSound():
\`\`\`
makeSound()
\`\`\`
makeAnimalSound(loudness?: number): string;
The name 'makeAnimalSound' is verbose. A shorter name like 'makeSound' conveys the purpose clearly.
More descriptive names improve readability.
4. Use boolean type for isMammal property:
4. Consider extending the interface from a base class for code reuse:
\`\`\`
isMammal: boolean
\`\`\`
export interface Animal extends BaseAnimal {
makeAnimalSound(loudness?: number): string;
}
Using the boolean type instead of just true/false improves readability.
Inheritance promotes abstraction and reduces duplication.
5. Add JSDoc comments for documentation:
5. Add JSDoc comments for overall interface description:
\`\`\`
/**
* Represents a creature in the game
* Represents an animal with common attributes.
*/
export interface Creature {
export interface Animal {
// ...
}
\`\`\`
JSDoc comments enable generating API documentation and improve understandability.
Comments improve understanding of intent and usage.
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."
Overall, the code is well-structured but could benefit from some minor improvements like annotations, readonly properties, descriptive names, inheritance, and comments. No major issues were identified."
`,
explainPollyError
)
Expand Down
10 changes: 5 additions & 5 deletions lib/shared/src/chat/input/at-mentioned.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,22 +20,22 @@ export function getAtMentionedInputText(
queryEndsWithColon = false
): { newInput: string; newInputCaretPosition: number } | undefined {
const inputBeforeCaret = formInput.slice(0, inputCaretPosition) || ''
const inputAfterCaret = formInput.slice(inputCaretPosition)
const lastAtIndex = inputBeforeCaret.lastIndexOf('@')
if (lastAtIndex < 0 || !formInput.trim()) {
return undefined
}

if (/:\d+(-)?(\d+)?( )?$/.test(inputBeforeCaret)) {
// Add a space after inputBeforeCaret to formInput
const newInput = formInput.replace(inputBeforeCaret, `${inputBeforeCaret} `)
return { newInput, newInputCaretPosition: inputCaretPosition }
const newInput = `${inputBeforeCaret} ${inputAfterCaret.trimStart()}`
return { newInput, newInputCaretPosition: inputCaretPosition + 1 }
}

// Trims any existing @file text from the input.
const inputPrefix = inputBeforeCaret.slice(0, lastAtIndex)
const afterCaret = formInput.slice(inputCaretPosition)
const spaceAfterCaret = afterCaret.indexOf(' ')
const inputSuffix = !spaceAfterCaret ? afterCaret : afterCaret.slice(spaceAfterCaret)
const spaceAfterCaret = inputAfterCaret.indexOf(' ')
const inputSuffix = !spaceAfterCaret ? inputAfterCaret : inputAfterCaret.slice(spaceAfterCaret)
// Add empty space at the end to end the file matching process,
// if the query ends with colon, add a colon instead as it's used for initial range selection.
const colon = queryEndsWithColon ? ':' : ''
Expand Down
4 changes: 2 additions & 2 deletions lib/shared/src/chat/input/user-context.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,15 +85,15 @@ describe('verifyContextFilesFromInput', () => {
])
})

it('handles invalid line numbers gracefully', () => {
it('returns empty array for invalid line numbers', () => {
const input = '@foo.ts:5-1'
const contextFilesMap = new Map<string, ContextFileFile>([
['foo.ts', { uri: URI.file('foo.ts'), type: 'file' }],
])

const contextFiles = verifyContextFilesFromInput(input, contextFilesMap)

expect(contextFiles).toEqual([{ uri: URI.file('foo.ts'), type: 'file' }])
expect(contextFiles).toEqual([])
})

it('sets range property even if only start line number is included', () => {
Expand Down
8 changes: 3 additions & 5 deletions lib/shared/src/chat/input/user-context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,9 @@ export function verifyContextFilesFromInput(

// Get the line number behind the file name if there is one:
// SingleLines Example: foo/bar.ts:1 > 1
const singleLines = input.matchAll(new RegExp(atFileName + ':([0-9]+)(?!-)', 'g'))
const singleLines = input.matchAll(new RegExp(atFileName + ':(\\d+)(?!-)', 'g'))
// MultiLines example: foo/bar.ts:1-2 > 1, 2
const multiLines = input.matchAll(new RegExp(atFileName + ':([0-9]+)-([0-9]+)', 'g'))

const multiLines = input.matchAll(new RegExp(atFileName + ':(\\d+)-(\\d+)', 'g'))
// loop all matches and set the range property on the contextFile object
for (const match of [...Array.from(singleLines), ...Array.from(multiLines)]) {
const contextFileMatch = { ...contextFile }
Expand All @@ -62,9 +61,8 @@ export function verifyContextFilesFromInput(
start: { line: startLineNum, character: 0 },
end: { line: endLineNum, character: 0 },
}
userContextFiles.push(contextFileMatch)
}

userContextFiles.push(contextFileMatch)
}
}

Expand Down
23 changes: 1 addition & 22 deletions lib/ui/src/Chat.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import {
type ContextFile,
type Guardrails,
getContextFileDisplayText,
displayPath,
getAtMentionQuery,
getAtMentionedInputText,
isAtMention,
Expand All @@ -24,7 +23,7 @@ import type { CodeBlockMeta } from './chat/CodeBlocks'
import type { FileLinkProps } from './chat/components/EnhancedContext'
import type { SymbolLinkProps } from './chat/PreciseContext'
import { Transcript } from './chat/Transcript'
import { isDefaultCommandPrompts, type TranscriptItemClassNames } from './chat/TranscriptItem'
import type { TranscriptItemClassNames } from './chat/TranscriptItem'

import styles from './Chat.module.css'
import { ChatActions } from './chat/components/ChatActions'
Expand Down Expand Up @@ -262,21 +261,12 @@ export const Chat: React.FunctionComponent<ChatProps> = ({
// Users can toggle this feature via "shift" + "Meta(Mac)/Control" keys
const [enableNewChatMode, setEnableNewChatMode] = useState(false)

const [isLastItemCommand, setIsLastItemCommand] = useState(false)

const lastHumanMessageIndex = useMemo<number | undefined>(() => {
if (!transcript?.length) {
return undefined
}
const index = transcript.findLastIndex(msg => msg.speaker === 'human')

// TODO (bee) can be removed once we support editing command prompts.
// Used for displaying "Edit Last Message" chat action button
const lastDisplayText = transcript[index]?.displayText ?? ''
const isCustomCommand = !!lastDisplayText.startsWith('/')
const isCoreCommand = isDefaultCommandPrompts(lastDisplayText)
setIsLastItemCommand(isCustomCommand || isCoreCommand)

return index
}, [transcript])

Expand Down Expand Up @@ -366,16 +356,6 @@ export const Chat: React.FunctionComponent<ChatProps> = ({
(selected: ContextFile, queryEndsWithColon = false): void => {
const atRangeEndingRegex = /:\d+(-\d+)?$/
const inputBeforeCaret = formInput.slice(0, inputCaretPosition)
const isSettingRange = atRangeEndingRegex.test(inputBeforeCaret)
if (chatContextFiles.has(`@${displayPath(selected.uri)}`)) {
if (isSettingRange) {
// Add a space after inputBeforeCaret to formInput
setFormInput(formInput.replace(inputBeforeCaret, inputBeforeCaret + ' '))
setInputCaretPosition(formInput.length + 1)
resetContextSelection()
return
}
}

const fileDisplayText = getContextFileDisplayText(selected, inputBeforeCaret)
if (inputCaretPosition && fileDisplayText) {
Expand Down Expand Up @@ -861,7 +841,6 @@ export const Chat: React.FunctionComponent<ChatProps> = ({
isEmptyChat={transcript.length < 1}
isMessageInProgress={!!messageInProgress?.speaker}
isEditing={transcript.length > 1 && messageBeingEdited !== undefined}
disableEditLastMessage={isLastItemCommand}
onChatResetClick={onChatResetClick}
onCancelEditClick={() => setEditMessageState()}
onEditLastMessageClick={() => setEditMessageState(lastHumanMessageIndex)}
Expand Down
23 changes: 1 addition & 22 deletions lib/ui/src/chat/TranscriptItem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -94,11 +94,6 @@ export const TranscriptItem: React.FunctionComponent<
// A boolean indicating whether the current transcript item is the one being edited.
const isItemBeingEdited = beingEdited === index

// TODO (bee) can be removed once we support editing command prompts.
// A boolean indicating whether the current message is a known command input.
const isCommandInput =
message?.displayText?.startsWith('/') || isDefaultCommandPrompts(message?.text)

return (
<div
className={classNames(
Expand Down Expand Up @@ -131,7 +126,7 @@ export const TranscriptItem: React.FunctionComponent<
className={styles.feedbackEditButtonsContainer}
messageBeingEdited={index}
setMessageBeingEdited={setBeingEdited}
disabled={isCommandInput || isInEditingMode}
disabled={isInEditingMode}
/>
</header>
</div>
Expand Down Expand Up @@ -183,7 +178,6 @@ export const TranscriptItem: React.FunctionComponent<
contextFiles={message.contextFiles}
fileLinkComponent={fileLinkComponent}
className={transcriptActionClassName}
isCommand={isCommandInput}
/>
) : (
inProgress && <LoadingContext />
Expand Down Expand Up @@ -212,18 +206,3 @@ export const TranscriptItem: React.FunctionComponent<
</div>
)
})

// TODO: TO BE REMOVED
// This is a temporary workaround for disabling editing on the default chat commands.
const commandPrompts = {
explain:
'Explain what the selected code does in simple terms. Assume the audience is a beginner programmer who has just learned the language features and basic syntax. Focus on explaining: 1) The purpose of the code 2) What input(s) it takes 3) What output(s) it produces 4) How it achieves its purpose through the logic and algorithm. 5) Any important logic flows or data transformations happening. Use simple language a beginner could understand. Include enough detail to give a full picture of what the code aims to accomplish without getting too technical. Format the explanation in coherent paragraphs, using proper punctuation and grammar. Write the explanation assuming no prior context about the code is known. Do not make assumptions about variables or functions not shown in the shared code. Start the answer with the name of the code that is being explained.',
smell: `Please review and analyze the selected code and identify potential areas for improvement related to code smells, readability, maintainability, performance, security, etc. Do not list issues already addressed in the given code. Focus on providing up to 5 constructive suggestions that could make the code more robust, efficient, or align with best practices. For each suggestion, provide a brief explanation of the potential benefits. After listing any recommendations, summarize if you found notable opportunities to enhance the code quality overall or if the code generally follows sound design principles. If no issues found, reply 'There are no errors.'`,
}

export function isDefaultCommandPrompts(text?: string): boolean {
if (!text) {
return false
}
return Object.values(commandPrompts).includes(text)
}
4 changes: 1 addition & 3 deletions lib/ui/src/chat/components/ChatActions.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ export const ChatActions: React.FunctionComponent<{
onEditLastMessageClick: () => void
setInputFocus: (focus: boolean) => void
onRestoreLastChatClick?: () => void
disableEditLastMessage: boolean
}> = React.memo(function ContextFilesContent({
isEditing,
isEmptyChat,
Expand All @@ -23,7 +22,6 @@ export const ChatActions: React.FunctionComponent<{
onCancelEditClick,
onEditLastMessageClick,
setInputFocus,
disableEditLastMessage,
onRestoreLastChatClick,
isWebviewActive,
}) {
Expand Down Expand Up @@ -52,7 +50,7 @@ export const ChatActions: React.FunctionComponent<{
keybind: `${osIcon}K`,
onClick: onEditLastMessageClick,
focus: true,
when: !isEmptyChat && !isEditing && !disableEditLastMessage,
when: !isEmptyChat && !isEditing,
},
{
name: '← Return to Previous Chat',
Expand Down
10 changes: 2 additions & 8 deletions lib/ui/src/chat/components/EnhancedContext.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,7 @@ export const EnhancedContext: React.FunctionComponent<{
contextFiles: ContextFile[]
fileLinkComponent: React.FunctionComponent<FileLinkProps>
className?: string
isCommand: boolean
}> = React.memo(function ContextFilesContent({
contextFiles,
fileLinkComponent: FileLink,
className,
isCommand,
}) {
}> = React.memo(function ContextFilesContent({ contextFiles, fileLinkComponent: FileLink, className }) {
if (!contextFiles.length) {
return
}
Expand All @@ -42,7 +36,7 @@ export const EnhancedContext: React.FunctionComponent<{
// Check if the filteredFiles only contain local context (non-enhanced context).
const localContextType = ['user', 'selection', 'terminal', 'editor']
const localContextOnly = contextFiles.every(file => localContextType.includes(file.type))
const sparkle = localContextOnly || isCommand ? '' : '✨ '
const sparkle = localContextOnly ? '' : '✨ '
const prefix = sparkle + 'Context: '
// It checks if file.range exists first before accessing start and end.
// If range doesn't exist, it adds 0 lines for that file.
Expand Down
8 changes: 7 additions & 1 deletion vscode/src/commands/execute/explain.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { logDebug, type ContextFile } from '@sourcegraph/cody-shared'
import { logDebug, type ContextFile, displayPath } from '@sourcegraph/cody-shared'
import { getContextFileFromCursor } from '../context/selection'
import { getContextFileFromCurrentFile } from '../context/current-file'
import { type ExecuteChatArguments, executeChat } from './ask'
Expand Down Expand Up @@ -38,6 +38,12 @@ export async function explainCommand(
const currentFile = await getContextFileFromCurrentFile()
contextFiles.push(...currentFile)

const cs = currentSelection[0] ?? currentFile[0]
if (cs) {
const range = cs.range && `:${cs.range.start.line + 1}-${cs.range.end.line + 1}`
prompt = prompt.replace('the selected code', `@${displayPath(cs.uri)}${range ?? ''} `)
}

return {
text: prompt,
submitType: 'user-newchat',
Expand Down
Loading

0 comments on commit b3649f2

Please sign in to comment.