-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
[Theia AI] Terminal agent records its requests #14246
Conversation
fixed #14245 Signed-off-by: Jonas Helming <jhelming@eclipsesource.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Thanks!
There was just a linting error. My suggestions inline should hopefully resolve those.
const responseText = JSON.stringify(parsedResult.data.commands); | ||
this.recordingService.recordResponse({ | ||
agentId: this.id, | ||
sessionId, | ||
timestamp: Date.now(), | ||
requestId, | ||
response: responseText, | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const responseText = JSON.stringify(parsedResult.data.commands); | |
this.recordingService.recordResponse({ | |
agentId: this.id, | |
sessionId, | |
timestamp: Date.now(), | |
requestId, | |
response: responseText, | |
}); | |
this.recordingService.recordResponse({ | |
agentId: this.id, | |
sessionId, | |
timestamp: Date.now(), | |
requestId, | |
response: JSON.stringify(parsedResult.data.commands), | |
}); |
This is also to avoid the linting error:
@theia/ai-terminal: /home/runner/work/theia/theia/packages/ai-terminal/src/browser/ai-terminal-agent.ts
@theia/ai-terminal: 202:27 error 'responseText' is already declared in the upper scope on line 216 column 19
@typescript-eslint/no-shadow
@theia/ai-terminal:
@theia/ai-terminal: ✖ 1 problem (1 error, 0 warnings)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had this before, but especially with GPT 3.5 it "hanged" sometimes. I guess the error in parsing is not thrown correctly then, could this be?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Happens only in the case below as in this case, the result is always pasrsable, but I wanted to make it consistent
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I fixed the linting error with the new commit
const responseText = JSON.stringify(jsonResult); | ||
this.recordingService.recordResponse({ | ||
agentId: this.id, | ||
sessionId, | ||
timestamp: Date.now(), | ||
requestId, | ||
response: responseText | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const responseText = JSON.stringify(jsonResult); | |
this.recordingService.recordResponse({ | |
agentId: this.id, | |
sessionId, | |
timestamp: Date.now(), | |
requestId, | |
response: responseText | |
}); | |
this.recordingService.recordResponse({ | |
agentId: this.id, | |
sessionId, | |
timestamp: Date.now(), | |
requestId, | |
response: JSON.stringify(jsonResult) | |
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had this before, but especially with GPT 3.5 it "hanged" sometimes. I guess the error in parsing is not thrown correctly then, could this be?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I fixed the linting error with the new commit
Co-authored-by: Philip Langer <planger@eclipsesource.com>
Co-authored-by: Philip Langer <planger@eclipsesource.com>
Signed-off-by: Jonas Helming <jhelming@eclipsesource.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, looks good to me! 👍
fixed #14245
What it does
Terminal agent records its requests
How to test
Use the terminal agent and observe its requests being logged in the History View. Use a model with and one without structured output.
Follow-ups
Review checklist
Reminder for reviewers