feat(ui): llm conversation about error #34750
Conversation
| @@ -142,6 +142,16 @@ export async function installRootRedirect(server: HttpServer, traceUrls: string[ | |||
| server.routePath('/', (_, response) => { | |||
| response.statusCode = 302; | |||
| response.setHeader('Location', urlPath); | |||
|
|
|||
| if (process.env.OPENAI_API_KEY) | |||
There was a problem hiding this comment.
This appears to not work with PW_HMR.
| </div>} | ||
| <span style={{ position: 'absolute', right: '5px' }}> | ||
| {llmAvailable | ||
| ? <ToolbarButton onClick={() => setShowLLM(v => !v)} style={{ width: '96px', justifyContent: 'center' }} title='Fix with AI' className='copy-to-clipboard-text-button'>{showLLM ? 'Hide AI' : 'Fix with AI'}</ToolbarButton> |
There was a problem hiding this comment.
Hiding then pressing this button again should not re-execute the conversation (I believe it's sending all existing messages, plus the system prompt again as the last message). There should be a separate button somewhere to restart the conversation when AI mode is visible.
There was a problem hiding this comment.
I do feel strongly about this. It's surprising that showing a component sends a message to llm. I'd much prefer if the first send was right here, as a reaction to the Fix with AI button click. This way we'll also be able to make it only do that once.
There was a problem hiding this comment.
i've refactored it to create the conversation and send the first message only on click, not on component mount
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Test results for "tests 1"12 flaky38562 passed, 793 skipped Merge workflow run. |
| </div>} | ||
| <span style={{ position: 'absolute', right: '5px' }}> | ||
| {llmAvailable | ||
| ? <ToolbarButton onClick={() => setShowLLM(v => !v)} style={{ width: '96px', justifyContent: 'center' }} title='Fix with AI' className='copy-to-clipboard-text-button'>{showLLM ? 'Hide AI' : 'Fix with AI'}</ToolbarButton> |
There was a problem hiding this comment.
I do feel strongly about this. It's surprising that showing a component sends a message to llm. I'd much prefer if the first send was right here, as a reaction to the Fix with AI button click. This way we'll also be able to make it only do that once.
packages/trace-viewer/src/ui/llm.tsx
Outdated
| const chat = useLLMChat(); | ||
| if (!chat) | ||
| throw new Error('No LLM chat available, make sure theres a LLMProvider above'); | ||
| const conversation = React.useMemo(() => chat.getConversation(id, systemPrompt), [chat, id]); // eslint-disable-line react-hooks/exhaustive-deps |
There was a problem hiding this comment.
In my experience, linter was always correct and I was always incorrect, so I always doubt disabling this rule 😄
There was a problem hiding this comment.
turns out that a little refactoring made it disappear :D
package.json
Outdated
| @@ -102,6 +102,7 @@ | |||
| "node-stream-zip": "^1.15.0", | |||
| "react": "^18.1.0", | |||
| "react-dom": "^18.1.0", | |||
| "react-markdown": "^9.0.3", | |||
There was a problem hiding this comment.
Let's look for a smaller dependency.
There was a problem hiding this comment.
After briefly trying to adapt https://github.com/developit/snarkdown to write our own React component for this, I gave up and used https://github.com/quantizor/markdown-to-jsx instead. It's 6kb gzipped.
| onClick={() => { | ||
| if (!chat.getConversation(conversationId)) { | ||
| const conversation = chat.startConversation(conversationId, [ | ||
| `My Playwright test failed. What's going wrong?`, |
There was a problem hiding this comment.
Is this different from fixTestPrompt? I feel like we should probably reuse them?
There was a problem hiding this comment.
It is different, because we can make a difference between system prompts, visible prompt contents and actual prompt contents. None of that works with the copy prompt buttons.
Screen.Recording.2025-02-12.at.14.40.31.mov