Skip to content

feat: add LSP extension for sending telemetry for inline chat result action #408

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.

Sign up for GitHub

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

Merged
pras0131 merged 9 commits into from
Apr 16, 2025
Merged

feat: add LSP extension for sending telemetry for inline chat result action #408

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
c064079
feat: add LSP extension for sending telemetry for inline chat result …
Apr 3, 2025
77bce83
Merge branch 'main' into addExtensionToEmitInlineChatMetric
pras0131 Apr 9, 2025
9f457a8
fix: correct formatting
Apr 9, 2025
50d3b14
Merge branch 'main' into addExtensionToEmitInlineChatMetric
Apr 11, 2025
dc75d6d
feat: update the naming from logInlineChatResult to inlineChatResult
Apr 11, 2025
bd9a068
Merge branch 'main' into addExtensionToEmitInlineChatMetric
Apr 15, 2025
3d24963
fix: full variable names, removed timestamp and added doc string for …
Apr 15, 2025
d4aadeb
Merge branch 'main' into addExtensionToEmitInlineChatMetric
Apr 16, 2025
c7018c5
fix: refactored tha InlineChatResultParams field names
Apr 16, 2025
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
5 changes: 5 additions & 0 deletions runtimes/protocol/chat.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ import {
ContextCommandParams,
CREATE_PROMPT_NOTIFICATION_METHOD,
CreatePromptParams,
INLINE_CHAT_RESULT_NOTIFICATION_METHOD,
InlineChatResultParams,
ListConversationsParams,
ListConversationsResult,
LIST_CONVERSATIONS_REQUEST_METHOD,
Expand Down Expand Up @@ -119,6 +121,9 @@ export const contextCommandsNotificationType = new ProtocolNotificationType<Cont
export const createPromptNotificationType = new ProtocolNotificationType<CreatePromptParams, void>(
CREATE_PROMPT_NOTIFICATION_METHOD
)
export const inlineChatResultNotificationType = new ProtocolNotificationType<InlineChatResultParams, void>(
INLINE_CHAT_RESULT_NOTIFICATION_METHOD
)

// history
export const listConversationsRequestType = new AutoParameterStructuresProtocolRequestType<
Expand Down
2 changes: 2 additions & 0 deletions runtimes/runtimes/base-runtime.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import {
inlineChatRequestType,
contextCommandsNotificationType,
createPromptNotificationType,
inlineChatResultNotificationType,
listConversationsRequestType,
conversationClickRequestType,
} from '../protocol'
Expand Down Expand Up @@ -150,6 +151,7 @@ export const baseRuntime = (connections: { reader: MessageReader; writer: Messag
onFileClicked: handler => lspConnection.onNotification(fileClickNotificationType.method, handler),
sendContextCommands: params => lspConnection.sendNotification(contextCommandsNotificationType.method, params),
onCreatePrompt: handler => lspConnection.onNotification(createPromptNotificationType.method, handler),
onInlineChatResult: handler => lspConnection.onNotification(inlineChatResultNotificationType.method, handler),
onListConversations: handler => lspConnection.onRequest(listConversationsRequestType.method, handler),
onConversationClick: handler => lspConnection.onRequest(conversationClickRequestType.method, handler),
}
Expand Down
6 changes: 6 additions & 0 deletions runtimes/runtimes/chat/baseChat.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ import {
contextCommandsNotificationType,
CreatePromptParams,
createPromptNotificationType,
InlineChatResultParams,
inlineChatResultNotificationType,
listConversationsRequestType,
ListConversationsParams,
ListConversationsResult,
Expand Down Expand Up @@ -133,6 +135,10 @@ export class BaseChat implements Chat {
this.connection.onNotification(createPromptNotificationType.method, handler)
}

public onInlineChatResult(handler: NotificationHandler<InlineChatResultParams>) {
this.connection.onNotification(inlineChatResultNotificationType.method, handler)
}

public onListConversations(handler: RequestHandler<ListConversationsParams, ListConversationsResult, void>) {
this.connection.onRequest(listConversationsRequestType.method, handler)
}
Expand Down
2 changes: 2 additions & 0 deletions runtimes/server-interface/chat.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import {
InlineChatResult,
ContextCommandParams,
CreatePromptParams,
InlineChatResultParams,
ListConversationsParams,
ListConversationsResult,
ConversationClickParams,
Expand Down Expand Up @@ -59,4 +60,5 @@ export type Chat = {
onFileClicked: (handler: NotificationHandler<FileClickParams>) => void
sendContextCommands: (params: ContextCommandParams) => void
onCreatePrompt: (handler: NotificationHandler<CreatePromptParams>) => void
onInlineChatResult: (handler: NotificationHandler<InlineChatResultParams>) => void
}
24 changes: 23 additions & 1 deletion types/chat.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ export const INLINE_CHAT_REQUEST_METHOD = 'aws/chat/sendInlineChatPrompt'
// context
export const CONTEXT_COMMAND_NOTIFICATION_METHOD = 'aws/chat/sendContextCommands'
export const CREATE_PROMPT_NOTIFICATION_METHOD = 'aws/chat/createPrompt'
export const INLINE_CHAT_RESULT_NOTIFICATION_METHOD = 'aws/chat/inlineChatResult'
// history
export const LIST_CONVERSATIONS_REQUEST_METHOD = 'aws/chat/listConversations'
export const CONVERSATION_CLICK_REQUEST_METHOD = 'aws/chat/conversationClick'
Expand Down Expand Up @@ -303,8 +304,29 @@ export interface CreatePromptParams {
promptName: string
}

// history
export interface ProgrammingLanguage {
languageName: string
}

export type InlineChatUserDecision = 'ACCEPT' | 'REJECT' | 'DISMISS' | string

export interface InlineChatResultParams {
Copy link
Copy Markdown
Contributor

@viktorsaws viktorsaws Apr 15, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

@pras0131 please document every new Protocol addition and meaning of all fields in doc strings. It should be very clear what is expected from client to pass here, and we should follow LSP spec example. You can see example of documentation I did in latest PR https://github.com/aws/language-server-runtimes/pull/433/files

It's difficult to reason about all fields in Params without documentation, it is not clear why some fields are needed.

Copy link
Copy Markdown
Contributor Author

@pras0131 pras0131 Apr 15, 2025

Choose a reason for hiding this comment

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

I can add a doc string to inlineChatResultNotificationType but adding a doc string to all field does not seem to be required here post the renaming of variables to fuller names as they are clearly indicate their purpose.

requestId: string
timestamp: Date
Copy link
Copy Markdown
Contributor

@viktorsaws viktorsaws Apr 15, 2025

Choose a reason for hiding this comment

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

What is this timestamp needed for and why it is required?

Copy link
Copy Markdown
Contributor Author

@pras0131 pras0131 Apr 15, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Same as above, required by InlineChatEvent but I do agree that we can just simply calculate it at server end instead of client passing it. This is the time when event is sent. Sample: here

inputLength?: number
numSelectedLines?: number
numSuggestionAddChars?: number
numSuggestionAddLines?: number
numSuggestionDelChars?: number
Copy link
Copy Markdown
Contributor

@viktorsaws viktorsaws Apr 15, 2025

Choose a reason for hiding this comment

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

It's better to use full words in protocol, it's clearer and more understandable.

numSuggestionDelLines?: number
codeIntent?: boolean
userDecision?: InlineChatUserDecision
responseStartLatency?: number
responseEndLatency?: number
Copy link
Copy Markdown
Contributor

@viktorsaws viktorsaws Apr 15, 2025

Choose a reason for hiding this comment

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

Why do we need 2 values for latency? Can client just return computed latency back?

Copy link
Copy Markdown
Contributor Author

@pras0131 pras0131 Apr 15, 2025

Choose a reason for hiding this comment

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

These different values are required while emitting Telemetry for InlineChatEvent.
Structure of InlineChatEvent: link

Sample usage in Eclipse: link

programmingLanguage?: ProgrammingLanguage
}

// history
export type TextBasedFilterOption = {
type: 'textarea' | 'textinput'
placeholder?: string
Expand Down