Skip to content

Commit 563ad73

Browse files
knightedcodemonkeyknightedcodemonkey
committed
refactor: address pr comments.
1 parent ea52ed2 commit 563ad73

2 files changed

Lines changed: 193 additions & 40 deletions

File tree

‎playwright/github-byot-ai.spec.ts‎

Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -556,6 +556,149 @@ test('AI chat apply actions resolve dynamic tab targets', async ({ page }) => {
556556
).toBeVisible()
557557
})
558558

559+
test('AI chat applies the correct proposal when unresolved targets are filtered out', async ({
560+
page,
561+
}) => {
562+
await page.route('https://models.github.ai/inference/chat/completions', async route => {
563+
const body = route.request().postDataJSON() as ChatRequestBody | null
564+
565+
if (body?.stream) {
566+
await route.fulfill({
567+
status: 502,
568+
contentType: 'application/json',
569+
body: JSON.stringify({ message: 'stream intentionally disabled in this test' }),
570+
})
571+
return
572+
}
573+
574+
await route.fulfill({
575+
status: 200,
576+
contentType: 'application/json',
577+
body: JSON.stringify({
578+
choices: [
579+
{
580+
message: {
581+
role: 'assistant',
582+
content: 'Prepared updates for App tab.',
583+
tool_calls: [
584+
{
585+
id: 'call_unresolved',
586+
type: 'function',
587+
function: {
588+
name: 'propose_editor_update',
589+
arguments: JSON.stringify({
590+
target: 'src/components/missing.tsx',
591+
content: 'const Missing = () => null',
592+
}),
593+
},
594+
},
595+
{
596+
id: 'call_component',
597+
type: 'function',
598+
function: {
599+
name: 'propose_editor_update',
600+
arguments: JSON.stringify({
601+
target: 'src/components/App.tsx',
602+
content: 'const App = () => <p>Resolved update</p>',
603+
}),
604+
},
605+
},
606+
],
607+
},
608+
},
609+
],
610+
}),
611+
})
612+
})
613+
614+
await waitForAppReady(page, `${appEntryPath}`)
615+
await connectByotWithSingleRepo(page)
616+
await setComponentEditorSource(page, 'const App = () => <p>Before</p>')
617+
await openWorkspaceTab(page, 'App.tsx')
618+
await ensureAiChatDrawerOpen(page)
619+
620+
await page.getByLabel('Ask AI assistant').fill('Update App tab only.')
621+
await page.getByRole('button', { name: 'Send' }).click()
622+
623+
await expect(
624+
page.getByRole('button', { name: 'Apply update to App.tsx' }),
625+
).toBeVisible()
626+
await page.getByRole('button', { name: 'Apply update to App.tsx' }).click()
627+
628+
await expect(
629+
page.locator('.editor-panel[data-editor-kind="component"] .cm-content').first(),
630+
).toContainText('Resolved update')
631+
})
632+
633+
test('AI chat renders a single apply action for multiple targets resolving to the same tab', async ({
634+
page,
635+
}) => {
636+
await page.route('https://models.github.ai/inference/chat/completions', async route => {
637+
const body = route.request().postDataJSON() as ChatRequestBody | null
638+
639+
if (body?.stream) {
640+
await route.fulfill({
641+
status: 502,
642+
contentType: 'application/json',
643+
body: JSON.stringify({ message: 'stream intentionally disabled in this test' }),
644+
})
645+
return
646+
}
647+
648+
await route.fulfill({
649+
status: 200,
650+
contentType: 'application/json',
651+
body: JSON.stringify({
652+
choices: [
653+
{
654+
message: {
655+
role: 'assistant',
656+
content: 'Prepared updates for App tab.',
657+
tool_calls: [
658+
{
659+
id: 'call_component_id',
660+
type: 'function',
661+
function: {
662+
name: 'propose_editor_update',
663+
arguments: JSON.stringify({
664+
target: 'component',
665+
content: 'const App = () => <p>By id</p>',
666+
}),
667+
},
668+
},
669+
{
670+
id: 'call_component_path',
671+
type: 'function',
672+
function: {
673+
name: 'propose_editor_update',
674+
arguments: JSON.stringify({
675+
target: 'src/components/App.tsx',
676+
content: 'const App = () => <p>By path</p>',
677+
}),
678+
},
679+
},
680+
],
681+
},
682+
},
683+
],
684+
}),
685+
})
686+
})
687+
688+
await waitForAppReady(page, `${appEntryPath}`)
689+
await connectByotWithSingleRepo(page)
690+
await setComponentEditorSource(page, 'const App = () => <p>Before</p>')
691+
await openWorkspaceTab(page, 'App.tsx')
692+
await ensureAiChatDrawerOpen(page)
693+
694+
await page.getByLabel('Ask AI assistant').fill('Update App tab once.')
695+
await page.getByRole('button', { name: 'Send' }).click()
696+
697+
await expect(page.getByRole('button', { name: 'Apply update to App.tsx' })).toHaveCount(
698+
1,
699+
)
700+
})
701+
559702
test('AI chat sends the currently active tab when context is enabled', async ({
560703
page,
561704
}) => {

‎src/modules/github/chat/drawer.js‎

Lines changed: 50 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import {
2020
} from './active-tab-context.js'
2121
import { buildOutboundMessages as buildPayloadMessages } from './payload.js'
2222
import { editorProposalTools, toMessageEditorProposals } from './proposals.js'
23-
import { resolveWorkspaceTabTarget, toTargetKey } from './tab-target-resolver.js'
23+
import { resolveWorkspaceTabTarget } from './tab-target-resolver.js'
2424
import { createTabScopedUndoState } from './tab-scoped-undo-state.js'
2525

2626
const svgNamespace = 'http://www.w3.org/2000/svg'
@@ -387,35 +387,8 @@ export const createGitHubChatDrawer = ({
387387
body.textContent = message.content
388388
item.append(body)
389389

390-
const proposals =
391-
message.role === 'assistant'
392-
? toMessageEditorProposals(message, {
393-
fallbackTarget: getFallbackProposalTarget(),
394-
})
395-
: []
396-
const workspaceTabs = getWorkspaceTabs()
397-
const activeTabId = getActiveTabContext()?.id || ''
398-
const resolvedProposals = proposals
399-
.map(proposal => {
400-
const resolvedTab = resolveWorkspaceTabTarget({
401-
target: proposal.target,
402-
language: proposal.language,
403-
tabs: workspaceTabs,
404-
activeTabId,
405-
})
406-
407-
if (!resolvedTab) {
408-
return null
409-
}
410-
411-
const appliedKey = toTargetKey(proposal.target)
412-
return {
413-
...proposal,
414-
appliedKey,
415-
resolvedTab,
416-
}
417-
})
418-
.filter(Boolean)
390+
const resolvedProposals =
391+
message.role === 'assistant' ? resolveMessageProposals(message) : []
419392
const hasProposal = resolvedProposals.length > 0
420393
const appliedTargets =
421394
message && typeof message.appliedTargets === 'object' && message.appliedTargets
@@ -427,13 +400,13 @@ export const createGitHubChatDrawer = ({
427400
actions.className = 'ai-chat-message__actions'
428401
actions.dataset.messageIndex = String(index)
429402

430-
const buildApplyButton = ({ proposal, proposalIndex }) => {
403+
const buildApplyButton = ({ proposal }) => {
431404
const button = document.createElement('button')
432405
button.type = 'button'
433406
button.className = 'render-button render-button--small ai-chat-message__action'
434407
button.dataset.action = 'request-apply'
435408
button.dataset.messageIndex = String(index)
436-
button.dataset.proposalIndex = String(proposalIndex)
409+
button.dataset.proposalOriginalIndex = String(proposal.proposalOriginalIndex)
437410
const tabLabel =
438411
proposal.resolvedTab.name ||
439412
proposal.resolvedTab.path ||
@@ -446,15 +419,22 @@ export const createGitHubChatDrawer = ({
446419
return button
447420
}
448421

449-
for (const [proposalIndex, proposal] of resolvedProposals.entries()) {
422+
const renderedApplyKeys = new Set()
423+
424+
for (const proposal of resolvedProposals) {
450425
if (!proposal?.appliedKey || appliedTargets[proposal.appliedKey] === true) {
451426
continue
452427
}
453428

429+
if (renderedApplyKeys.has(proposal.appliedKey)) {
430+
continue
431+
}
432+
433+
renderedApplyKeys.add(proposal.appliedKey)
434+
454435
actions.append(
455436
buildApplyButton({
456437
proposal,
457-
proposalIndex,
458438
}),
459439
)
460440
}
@@ -529,7 +509,37 @@ export const createGitHubChatDrawer = ({
529509
return `${nextValue}\n`
530510
}
531511

532-
const applyProposalToTab = ({ messageIndex, proposalIndex }) => {
512+
const resolveMessageProposals = message => {
513+
const proposals = toMessageEditorProposals(message, {
514+
fallbackTarget: getFallbackProposalTarget(),
515+
})
516+
const workspaceTabs = getWorkspaceTabs()
517+
const activeTabId = getActiveTabContext()?.id || ''
518+
519+
return proposals
520+
.map((proposal, proposalOriginalIndex) => {
521+
const resolvedTab = resolveWorkspaceTabTarget({
522+
target: proposal.target,
523+
language: proposal.language,
524+
tabs: workspaceTabs,
525+
activeTabId,
526+
})
527+
528+
if (!resolvedTab) {
529+
return null
530+
}
531+
532+
return {
533+
...proposal,
534+
proposalOriginalIndex,
535+
appliedKey: resolvedTab.id,
536+
resolvedTab,
537+
}
538+
})
539+
.filter(Boolean)
540+
}
541+
542+
const applyProposalToTab = ({ messageIndex, proposalOriginalIndex }) => {
533543
const message = messages[messageIndex]
534544
if (!message || message.role !== 'assistant') {
535545
return null
@@ -538,7 +548,7 @@ export const createGitHubChatDrawer = ({
538548
const proposals = toMessageEditorProposals(message, {
539549
fallbackTarget: getFallbackProposalTarget(),
540550
})
541-
const proposal = proposals[proposalIndex]
551+
const proposal = proposals[proposalOriginalIndex]
542552
if (!proposal) {
543553
return null
544554
}
@@ -583,7 +593,7 @@ export const createGitHubChatDrawer = ({
583593
const tabLabel = resolvedTab.name || resolvedTab.path || resolvedTab.id
584594
setChatStatus(`Applied assistant proposal to ${tabLabel}.`, 'ok')
585595
return {
586-
appliedKey: toTargetKey(proposal.target),
596+
appliedKey: resolvedTab.id,
587597
tabId: resolvedTab.id,
588598
}
589599
}
@@ -946,14 +956,14 @@ export const createGitHubChatDrawer = ({
946956
}
947957

948958
if (action === 'request-apply') {
949-
const proposalIndex = Number(button.dataset.proposalIndex)
950-
if (!Number.isFinite(proposalIndex) || proposalIndex < 0) {
959+
const proposalOriginalIndex = Number(button.dataset.proposalOriginalIndex)
960+
if (!Number.isFinite(proposalOriginalIndex) || proposalOriginalIndex < 0) {
951961
return
952962
}
953963

954964
const applied = applyProposalToTab({
955965
messageIndex,
956-
proposalIndex,
966+
proposalOriginalIndex,
957967
})
958968

959969
if (!applied) {

0 commit comments

Comments
 (0)