Fixed garbage collection of aborted tasks; handle if run from root directory then don’t read/write; fixed scroll to bottom; fix other small bugs

This commit is contained in:
Saoud Rizwan
2024-07-09 23:44:20 -04:00
parent 9867a6a597
commit 7170d2a2e8
8 changed files with 101 additions and 26 deletions

View File

@@ -175,6 +175,7 @@ export class ClaudeDev {
private askResponse?: ClaudeAskResponse
private askResponseText?: string
private providerRef: WeakRef<SidebarProvider>
abort: boolean = false
constructor(provider: SidebarProvider, task: string, apiKey: string, maxRequestsPerTask?: number) {
this.providerRef = new WeakRef(provider)
@@ -198,6 +199,8 @@ export class ClaudeDev {
}
async ask(type: ClaudeAsk, question: string): Promise<{ response: ClaudeAskResponse; text?: string }> {
// If this ClaudeDev instance was aborted by the provider, then the only thing keeping us alive is a promise still running in the background, in which case we don't want to send its result to the webview as it is attached to a new instance of ClaudeDev now. So we can safely ignore the result of any active promises, and this class will be deallocated. (Although we set claudeDev = undefined in provider, that simply removes the reference to this instance, but the instance is still alive until this promise resolves or rejects.)
if (this.abort) { throw new Error("ClaudeDev instance aborted") }
this.askResponse = undefined
this.askResponseText = undefined
await this.providerRef.deref()?.addClaudeMessage({ ts: Date.now(), type: "ask", ask: type, text: question })
@@ -210,6 +213,7 @@ export class ClaudeDev {
}
async say(type: ClaudeSay, text: string): Promise<undefined> {
if (this.abort) { throw new Error("ClaudeDev instance aborted") }
await this.providerRef.deref()?.addClaudeMessage({ ts: Date.now(), type: "say", say: type, text: text })
await this.providerRef.deref()?.postStateToWebview()
}
@@ -385,7 +389,7 @@ ${activeEditorContents}`
if (shouldLog) {
this.say("tool", JSON.stringify({ tool: "listFiles", path: dirPath, content: "/" } as ClaudeSayTool))
}
return "Currently in the root directory. Cannot list all files."
return "WARNING: You are currently in the root directory! You DO NOT have read or write permissions in this directory, so you would need to use a command like \`echo $HOME\` to find a path you can work with (e.g. the user's Desktop directory). If you cannot accomplish your task in the root directory, you need to tell the user to \"open this extension in a workspace or another directory\" (since you are a script being run in a VS Code extension)."
}
try {
@@ -459,6 +463,8 @@ ${activeEditorContents}`
| Anthropic.ToolResultBlockParam
>
): Promise<ClaudeRequestResult> {
if (this.abort) { throw new Error("ClaudeDev instance aborted") }
this.conversationHistory.push({ role: "user", content: userContent })
if (this.requestCount >= this.maxRequestsPerTask) {
const { response } = await this.ask(
@@ -483,7 +489,16 @@ ${activeEditorContents}`
}
try {
await this.say("api_req_started", JSON.stringify({ request: userContent }))
// what the user sees in the webview
await this.say("api_req_started", JSON.stringify({ request: {
model: "claude-3-5-sonnet-20240620",
max_tokens: 4096,
system: "(see SYSTEM_PROMPT in src/ClaudeDev.ts)",
messages: [{ "conversation_history": "..." }, { role: "user", content: userContent }],
tools: "(see tools in src/ClaudeDev.ts)",
tool_choice: { type: "auto" },
}}))
const response = await this.client.messages.create({
model: "claude-3-5-sonnet-20240620", // https://docs.anthropic.com/en/docs/about-claude/models
max_tokens: 4096,

View File

@@ -220,7 +220,10 @@ export class SidebarProvider implements vscode.WebviewViewProvider {
}
async clearTask() {
this.claudeDev = undefined
if (this.claudeDev) {
this.claudeDev.abort = true // will stop any agentically running promises
this.claudeDev = undefined // removes reference to it, so once promises end it will be garbage collected
}
await this.setClaudeMessages([])
}

View File

@@ -19,6 +19,7 @@
"react": "^18.3.1",
"react-dom": "^18.3.1",
"react-scripts": "5.0.1",
"react-scroll": "^1.9.0",
"react-syntax-highlighter": "^15.5.0",
"react-text-truncate": "^0.19.0",
"react-textarea-autosize": "^8.5.3",
@@ -27,6 +28,7 @@
"web-vitals": "^2.1.4"
},
"devDependencies": {
"@types/react-scroll": "^1.8.10",
"@types/react-syntax-highlighter": "^15.5.13",
"@types/react-text-truncate": "^0.14.4",
"@types/vscode-webview": "^1.57.5"
@@ -4575,6 +4577,16 @@
"@types/react": "*"
}
},
"node_modules/@types/react-scroll": {
"version": "1.8.10",
"resolved": "https://registry.npmjs.org/@types/react-scroll/-/react-scroll-1.8.10.tgz",
"integrity": "sha512-RD4Z7grbdNGOKwKnUBKar6zNxqaW3n8m9QSrfvljW+gmkj1GArb8AFBomVr6xMOgHPD3v1uV3BrIf01py57daQ==",
"dev": true,
"license": "MIT",
"dependencies": {
"@types/react": "*"
}
},
"node_modules/@types/react-syntax-highlighter": {
"version": "15.5.13",
"resolved": "https://registry.npmjs.org/@types/react-syntax-highlighter/-/react-syntax-highlighter-15.5.13.tgz",
@@ -13448,6 +13460,12 @@
"integrity": "sha512-HDWXG8isMntAyRF5vZ7xKuEvOhT4AhlRt/3czTSjvGUxjYCBVRQY48ViDHyfYz9VIoBkW4TMGQNapx+l3RUwdA==",
"license": "MIT"
},
"node_modules/lodash.throttle": {
"version": "4.1.1",
"resolved": "https://registry.npmjs.org/lodash.throttle/-/lodash.throttle-4.1.1.tgz",
"integrity": "sha512-wIkUCfVKpVsWo3JSZlc+8MB5it+2AN5W8J7YVMST30UrvcQNZ1Okbj+rbVniijTWE6FGYy4XJq/rHkas8qJMLQ==",
"license": "MIT"
},
"node_modules/lodash.uniq": {
"version": "4.5.0",
"resolved": "https://registry.npmjs.org/lodash.uniq/-/lodash.uniq-4.5.0.tgz",
@@ -16377,6 +16395,20 @@
}
}
},
"node_modules/react-scroll": {
"version": "1.9.0",
"resolved": "https://registry.npmjs.org/react-scroll/-/react-scroll-1.9.0.tgz",
"integrity": "sha512-mamNcaX9Ng+JeSbBu97nWwRhYvL2oba+xR2GxvyXsbDeGP+gkYIKZ+aDMMj/n20TbV9SCWm/H7nyuNTSiXA6yA==",
"license": "MIT",
"dependencies": {
"lodash.throttle": "^4.1.1",
"prop-types": "^15.7.2"
},
"peerDependencies": {
"react": "^15.5.4 || ^16.0.0 || ^17.0.0 || ^18.0.0",
"react-dom": "^15.5.4 || ^16.0.0 || ^17.0.0 || ^18.0.0"
}
},
"node_modules/react-syntax-highlighter": {
"version": "15.5.0",
"resolved": "https://registry.npmjs.org/react-syntax-highlighter/-/react-syntax-highlighter-15.5.0.tgz",

View File

@@ -14,6 +14,7 @@
"react": "^18.3.1",
"react-dom": "^18.3.1",
"react-scripts": "5.0.1",
"react-scroll": "^1.9.0",
"react-syntax-highlighter": "^15.5.0",
"react-text-truncate": "^0.19.0",
"react-textarea-autosize": "^8.5.3",
@@ -46,6 +47,7 @@
]
},
"devDependencies": {
"@types/react-scroll": "^1.8.10",
"@types/react-syntax-highlighter": "^15.5.13",
"@types/react-text-truncate": "^0.14.4",
"@types/vscode-webview": "^1.57.5"

View File

@@ -6,7 +6,7 @@ import SettingsView from "./components/SettingsView"
import { ClaudeMessage, ExtensionMessage } from "@shared/ExtensionMessage"
import WelcomeView from "./components/WelcomeView"
import { vscode } from "./utilities/vscode"
import { mockMessages } from "./utilities/mockMessages"
//import { mockMessages } from "./utilities/mockMessages"
/*
The contents of webviews however are created when the webview becomes visible and destroyed when the webview is moved into the background. Any state inside the webview will be lost when the webview is moved to a background tab.
@@ -74,7 +74,7 @@ const App: React.FC = () => {
onDone={() => setShowSettings(false)}
/>
) : (
<ChatView messages={mockMessages} />
<ChatView messages={claudeMessages} />
)}
</>
)

View File

@@ -256,7 +256,6 @@ const ChatRow: React.FC<ChatRowProps> = ({ message }) => {
<CodeBlock
code={output}
language="shell-session"
path="src/components/WelcomeView.tsx/src/components/WelcomeView.tsx"
/>
</>
)}
@@ -295,19 +294,24 @@ const ChatRow: React.FC<ChatRowProps> = ({ message }) => {
}
}
// we need to return null here instead of in getContent since that way would result in padding being applied
if (message.say === "api_req_finished") {
return null // Don't render anything for this message type
}
if (message.type === "ask" && message.ask === "completion_result" && message.text === "") {
return null // Don't render anything for this message type
}
return (
<div
style={{
padding: "10px 5px 10px 20px",
padding: "10px 20px 10px 20px",
}}>
{renderContent()}
{isExpanded && message.say === "api_req_started" && (
<div style={{ marginTop: "10px" }}>
<CodeBlock code={JSON.stringify(JSON.parse(message.text || "{}").request)} language="json" />
<CodeBlock code={JSON.stringify(JSON.parse(message.text || "{}").request, null, 2)} language="json" />
</div>
)}
</div>

View File

@@ -9,19 +9,20 @@ import { combineCommandSequences } from "../utilities/combineCommandSequences"
import { combineApiRequests } from "../utilities/combineApiRequests"
import TaskHeader from "./TaskHeader"
import { getApiMetrics } from "../utilities/getApiMetrics"
import { animateScroll as scroll } from "react-scroll"
interface ChatViewProps {
messages: ClaudeMessage[]
}
// maybe instead of storing state in App, just make chatview always show so dont conditionally load/unload? need to make sure messages are persisted (i remember seeing something about how webviews can be frozen in docs)
const ChatView = ({ messages }: ChatViewProps) => {
const task = messages.shift()
const modifiedMessages = useMemo(() => combineApiRequests(combineCommandSequences(messages)), [messages])
//const task = messages.length > 0 ? (messages[0].say === "task" ? messages[0] : undefined) : undefined
const task = messages.length > 0 ? messages[0] : undefined // leaving this less safe version here since if the first message is not a task, then the extension is in a bad state and needs to be debugged (see ClaudeDev.abort)
const modifiedMessages = useMemo(() => combineApiRequests(combineCommandSequences(messages.slice(1))), [messages])
// has to be after api_req_finished are all reduced into api_req_started messages
const apiMetrics = useMemo(() => getApiMetrics(modifiedMessages), [modifiedMessages])
const [inputValue, setInputValue] = useState("")
const messagesEndRef = useRef<HTMLDivElement>(null)
const textAreaRef = useRef<HTMLTextAreaElement>(null)
const [textAreaHeight, setTextAreaHeight] = useState<number | undefined>(undefined)
const [textAreaDisabled, setTextAreaDisabled] = useState(false)
@@ -33,12 +34,12 @@ const ChatView = ({ messages }: ChatViewProps) => {
const [secondaryButtonText, setSecondaryButtonText] = useState<string | undefined>(undefined)
const scrollToBottom = (instant: boolean = false) => {
// https://stackoverflow.com/questions/11039885/scrollintoview-causing-the-whole-page-to-move
;(messagesEndRef.current as any)?.scrollIntoView({
behavior: instant ? "instant" : "smooth",
block: "nearest",
inline: "start",
})
const options = {
containerId: "chat-view-container",
duration: instant ? 0 : 500,
smooth: !instant,
}
scroll.scrollToBottom(options)
}
// scroll to bottom when new message is added
@@ -50,8 +51,13 @@ const ChatView = ({ messages }: ChatViewProps) => {
[modifiedMessages]
)
useEffect(() => {
scrollToBottom()
}, [visibleMessages.length])
const timer = setTimeout(() => {
scrollToBottom()
}, 0)
return () => {
clearTimeout(timer)
}
}, [visibleMessages])
useEffect(() => {
// if last message is an ask, show user ask UI
@@ -114,13 +120,23 @@ const ChatView = ({ messages }: ChatViewProps) => {
break
}
} else {
// this would get called after sending the first message, so we have to watch messages.length instead
// No messages, so user has to submit a task
// setTextAreaDisabled(false)
// setClaudeAsk(undefined)
// setPrimaryButtonText(undefined)
// setSecondaryButtonText(undefined)
}
}, [messages])
useEffect(() => {
if (messages.length === 0) {
setTextAreaDisabled(false)
setClaudeAsk(undefined)
setPrimaryButtonText(undefined)
setSecondaryButtonText(undefined)
}
}, [messages])
}, [messages.length])
const handleSendMessage = () => {
const text = inputValue.trim()
@@ -249,6 +265,7 @@ const ChatView = ({ messages }: ChatViewProps) => {
/>
)}
<div
id="chat-view-container"
className="scrollable"
style={{
flexGrow: 1,
@@ -257,7 +274,6 @@ const ChatView = ({ messages }: ChatViewProps) => {
{modifiedMessages.map((message, index) => (
<ChatRow key={index} message={message} />
))}
<div style={{ float: "left", clear: "both" }} ref={messagesEndRef} />
</div>
{(primaryButtonText || secondaryButtonText) && (
<div style={{ display: "flex", padding: "10px 15px 0px 15px" }}>
@@ -282,7 +298,7 @@ const ChatView = ({ messages }: ChatViewProps) => {
)}
</div>
)}
<div style={{ padding: "10px 15px" }}>
<div style={{ padding: "10px 15px", opacity: textAreaDisabled ? 0.7 : 1 }}>
<DynamicTextArea
ref={textAreaRef}
value={inputValue}
@@ -305,20 +321,24 @@ const ChatView = ({ messages }: ChatViewProps) => {
lineHeight: "var(--vscode-editor-line-height)",
resize: "none",
overflow: "hidden",
padding: "8px 40px 8px 8px",
padding: "8px 36px 8px 8px",
}}
/>
{textAreaHeight && (
<div
style={{
position: "absolute",
right: "18px",
right: "20px",
height: `${textAreaHeight}px`,
bottom: "12px",
display: "flex",
alignItems: "center",
}}>
<VSCodeButton disabled={textAreaDisabled} appearance="icon" aria-label="Send Message" onClick={handleSendMessage}>
<VSCodeButton
disabled={textAreaDisabled}
appearance="icon"
aria-label="Send Message"
onClick={handleSendMessage}>
<span className="codicon codicon-send"></span>
</VSCodeButton>
</div>

View File

@@ -89,7 +89,6 @@ const CodeBlock = ({ code, diff, language, path }: CodeBlockProps) => {
<div
style={{
borderRadius: "3px",
marginRight: "2px",
backgroundColor: backgroundColor,
overflow: "hidden", // This ensures the inner scrollable area doesn't overflow the rounded corners
}}>