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

Improve Cody logging agent protocol #6069

Merged
merged 3 commits into from
Nov 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ object Constants {
const val `class` = "class"
const val `client-managed` = "client-managed"
const val `create-file` = "create-file"
const val debug = "debug"
const val default = "default"
const val delete = "delete"
const val `delete-file` = "delete-file"
Expand Down Expand Up @@ -84,6 +85,7 @@ object Constants {
const val symbol = "symbol"
const val system = "system"
const val terminal = "terminal"
const val trace = "trace"
const val tree = "tree"
const val `tree-sitter` = "tree-sitter"
const val unauthenticated = "unauthenticated"
Expand All @@ -92,6 +94,7 @@ object Constants {
const val user = "user"
const val vision = "vision"
const val waitlist = "waitlist"
const val warn = "warn"
const val warning = "warning"
const val workspace = "workspace"
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,6 @@ package com.sourcegraph.cody.agent.protocol_generated;
data class DebugMessage(
val channel: String,
val message: String,
val level: DebugMessageLogLevel? = null, // Oneof: trace, debug, info, warn, error
)

Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
@file:Suppress("FunctionName", "ClassName", "unused", "EnumEntryName", "UnusedImport")
package com.sourcegraph.cody.agent.protocol_generated;

typealias DebugMessageLogLevel = String // One of: trace, debug, info, warn, error

1 change: 1 addition & 0 deletions agent/src/agent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,7 @@ export class Agent extends MessageHandler implements ExtensionClient {
this.notify('debug/message', {
channel: 'Document Sync Check',
message: panicMessage + '\n' + message,
level: 'error',
})
},
edit: (uri, callback, options) => {
Expand Down
42 changes: 27 additions & 15 deletions agent/src/vscode-shim.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ import { AgentTabGroups } from './AgentTabGroups'
import { AgentWorkspaceConfiguration } from './AgentWorkspaceConfiguration'
import type { Agent } from './agent'
import { matchesGlobPatterns } from './cli/command-bench/matchesGlobPatterns'
import type { ClientInfo, ExtensionConfiguration } from './protocol-alias'
import type { ClientInfo, DebugMessageLogLevel, ExtensionConfiguration } from './protocol-alias'

// Not using CODY_TESTING because it changes the URL endpoint we send requests
// to and we want to send requests to sourcegraph.com because we record the HTTP
Expand Down Expand Up @@ -530,32 +530,44 @@ export namespace UriString {
}

function outputChannel(name: string): vscode.LogOutputChannel {
function notifyAgent(level: DebugMessageLogLevel, message: string, ...args: any[]): void {
if (agent) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is some contradiction in this method. It is called notifyAgent but we seem to check if we use agent inside of it.

maybe s/notifyAgent/notifyAgentIfApplicable/g?

no hard feelings about it tho. feel free to leave it as it is if it makes sense to you

Copy link
Contributor Author

@pkukielka pkukielka Nov 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This check does not verify if we are in the 'agent' mode, it just check if agent object is already initialized.
vscode-shim.ts is used only for agent-related workflows.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, makes sense.

const formattedMessage = args.length
? String(message).replace(/{(\d+)}/g, (match, num) => args[num]?.toString() ?? match)
: message
agent.notify('debug/message', { channel: name, message: formattedMessage, level })
}
}
return {
name,
append: message => {
if (agent) {
agent.notify('debug/message', { channel: name, message })
}
notifyAgent('info', message)
},
appendLine: message => {
if (agent) {
agent.notify('debug/message', { channel: name, message })
}
notifyAgent('trace', message)
},
replace: message => {
if (agent) {
agent.notify('debug/message', { channel: name, message })
}
notifyAgent('trace', message)
},
clear: () => {},
show: () => {},
hide: () => {},
dispose: () => {},
trace: () => {},
debug: () => {},
info: () => {},
warn: () => {},
error: () => {},
trace: (message: string, ...args: any[]) => {
notifyAgent('trace', message, args)
},
debug: (message: string, ...args: any[]) => {
notifyAgent('debug', message, args)
},
info: (message: string, ...args: any[]) => {
notifyAgent('info', message, args)
},
warn: (message: string, ...args: any[]) => {
notifyAgent('warn', message, args)
},
error: (message: string, ...args: any[]) => {
notifyAgent('error', message, args)
},
logLevel: LogLevel.Trace,
onDidChangeLogLevel: emptyEvent(),
}
Expand Down
3 changes: 3 additions & 0 deletions vscode/src/jsonrpc/agent-protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -767,9 +767,12 @@ interface ExecuteCommandParams {
arguments?: any[] | undefined | null
}

export type DebugMessageLogLevel = 'trace' | 'debug' | 'info' | 'warn' | 'error'

export interface DebugMessage {
channel: string
message: string
level?: DebugMessageLogLevel | undefined | null
}

export interface ProgressStartParams {
Expand Down
Loading