mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-10-28 08:02:43 +00:00
feat: add ai review to fork deploys and deployment requests (#12029)
* feat: add ai review to deployment requests Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix: bound ai review fetches and surface unreadable items Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * feat: add fix in ai session and model picker to deployment ai review Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * feat: add workspace instructions for deployment ai review Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix: read review instructions from the parent and bind ai review to the fork Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix: pin the fix-all session preview to the fork's compare page Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor: drop workspace-settings instructions from deployment ai review Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * feat: review the selected items with ai on compare and deploy Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix: mark an ai review out of date when its items change Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * feat: post all ai review findings as comments at once Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * chore: update ee-repo-ref to 77703cf32e58964906d87ee83ddda44b5b8d1b7e This commit updates the EE repository reference after PR #926 was merged in windmill-ee-private. Previous ee-repo-ref: e30ab9e69d1854fcaed5620c934ace32acb0a729 New ee-repo-ref: 77703cf32e58964906d87ee83ddda44b5b8d1b7e Automated by sync-ee-ref workflow. --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: windmill-internal-app[bot] <windmill-internal-app[bot]@users.noreply.github.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
windmill-internal-app[bot]
parent
b813c1b35d
commit
bac1e463e5
@@ -1 +1 @@
|
||||
747336910e5c7498544613ad7863cea59f4db6a1
|
||||
77703cf32e58964906d87ee83ddda44b5b8d1b7e
|
||||
|
||||
@@ -4,12 +4,13 @@
|
||||
anonymous usage-stats payload. It answers "does anyone use this, and which variant do they pick"
|
||||
without any identifying data leaving the instance.
|
||||
|
||||
It currently carries 78 registered actions across twenty-nine features (`ai_session`, `ai_chat`,
|
||||
It currently carries 86 registered actions across thirty-three features (`ai_session`, `ai_chat`,
|
||||
`ai_fix`, `ai_agent`, `ai_decision`, `ai_agent_eval`, `app_sandbox`, `datatable`, `db_manager`, `flow_editor`,
|
||||
`flow_run`, `flow_step`, `home`, `header_nav`, `run_form`, `debugger`, `trigger`, `command_script`,
|
||||
`hub_script`, `usage_meter`, `sso_groups_claim`, `cloud_trial_offer`, `job_token_scopes`,
|
||||
`worker_draining`, `worker_group`, `worker_resource_stats`, `dev_workspace_isolation`,
|
||||
`slack_agent`, `variable_expiration`). Nearly all of the
|
||||
`mcp`, `mcp_workspace`, `hub_script`, `usage_meter`, `sso_groups_claim`, `cloud_trial_offer`,
|
||||
`job_token_scopes`, `worker_draining`, `worker_group`, `worker_resource_stats`,
|
||||
`dev_workspace_isolation`, `slack_agent`, `variable_expiration`, `anthropic_federation`,
|
||||
`deployment_request`). Nearly all of the
|
||||
product is uninstrumented, so new user-facing work is the opportunity to change that.
|
||||
|
||||
## When to instrument
|
||||
|
||||
@@ -65,6 +65,8 @@
|
||||
import { deepEqual } from 'fast-equals'
|
||||
import WorkspaceDeployLayout from './WorkspaceDeployLayout.svelte'
|
||||
import DeploymentRequestPanel from './deploymentRequest/DeploymentRequestPanel.svelte'
|
||||
import AiReview from './deploymentRequest/AiReview.svelte'
|
||||
import { copilotInfo } from '$lib/aiStore'
|
||||
import { userStore } from '$lib/stores'
|
||||
import CompareModeToggle, { type CompareMode } from './CompareModeToggle.svelte'
|
||||
import CompareTargetPicker from './CompareTargetPicker.svelte'
|
||||
@@ -430,6 +432,7 @@
|
||||
)
|
||||
|
||||
let selectedItems = $state<string[]>([])
|
||||
let selectedDiffs = $derived(selectableDiffs.filter((d) => selectedItems.includes(getItemKey(d))))
|
||||
|
||||
// Selected items that carry a pending draft. They're opt-in (excluded from the
|
||||
// default selection), so a non-empty list means the user explicitly picked an
|
||||
@@ -1807,8 +1810,18 @@
|
||||
|
||||
{#snippet footer()}
|
||||
{#if !nothingToAct}
|
||||
<div class="flex items-center justify-between">
|
||||
<div></div>
|
||||
<div class="flex items-start justify-between gap-4">
|
||||
<div class="flex-1 min-w-0">
|
||||
<!-- The request form and an open request carry their own review. -->
|
||||
{#if mergeIntoParent && $copilotInfo.enabled && !hasOpenDeploymentRequest && !deploymentRequestPanel?.isDialogOpen()}
|
||||
<AiReview
|
||||
diffs={selectedDiffs}
|
||||
forkWorkspaceId={currentWorkspaceId}
|
||||
{parentWorkspaceId}
|
||||
source="selection"
|
||||
/>
|
||||
{/if}
|
||||
</div>
|
||||
|
||||
<div class="flex flex-col items-end gap-2">
|
||||
<!-- Always show the deploy footer: only the items visible to the user are
|
||||
@@ -1885,6 +1898,7 @@
|
||||
{parentWorkspaceId}
|
||||
currentUsername={$userStore?.username ?? ''}
|
||||
isAdmin={$userStore?.is_admin ?? false}
|
||||
diffs={comparison?.diffs.filter((d) => diffActionableInDirection(d, true)) ?? []}
|
||||
onStateChange={(open) => {
|
||||
hasOpenDeploymentRequest = open
|
||||
}}
|
||||
|
||||
@@ -0,0 +1,273 @@
|
||||
<script lang="ts">
|
||||
import { Alert, Badge, Button } from '$lib/components/common'
|
||||
import type { AIProviderModel, WorkspaceItemDiff } from '$lib/gen'
|
||||
import { copilotInfo, copilotSessionModel } from '$lib/aiStore'
|
||||
import Select from '$lib/components/select/Select.svelte'
|
||||
import { sendUserToast } from '$lib/toast'
|
||||
import { logFeatureUsage } from '$lib/utils/featureUsage'
|
||||
import { Loader2, MessageSquarePlus, Sparkles, WandSparkles, X } from 'lucide-svelte'
|
||||
import OpenInSessionButton, {
|
||||
type OpenInSessionSource
|
||||
} from '$lib/components/sessions/OpenInSessionButton.svelte'
|
||||
import { AIBtnClasses } from '$lib/components/copilot/chat/AIButtonStyle'
|
||||
import { buildCompareUrl } from '$lib/components/copilot/chat/global/pageNavigation'
|
||||
import type { SessionTarget } from '$lib/components/sessions/sessionState.svelte'
|
||||
import { reviewDeployment, type AiReviewFinding, type AiReviewResult } from './aiReview'
|
||||
|
||||
interface Props {
|
||||
diffs: WorkspaceItemDiff[]
|
||||
forkWorkspaceId: string
|
||||
parentWorkspaceId: string
|
||||
/** Where the review runs: the items selected for deploy, the request form, or an open request. */
|
||||
source: 'selection' | 'request_form' | 'open_request'
|
||||
/** Absent before the request exists: findings are then only shown, for the author to act on. */
|
||||
onPost?: (finding: AiReviewFinding) => Promise<void>
|
||||
}
|
||||
|
||||
let { diffs, forkWorkspaceId, parentWorkspaceId, source, onPost }: Props = $props()
|
||||
|
||||
let running = $state(false)
|
||||
let result = $state<AiReviewResult | undefined>(undefined)
|
||||
let error = $state<string | undefined>(undefined)
|
||||
let postingId = $state<string | undefined>(undefined)
|
||||
let abortController: AbortController | undefined
|
||||
|
||||
// `diffs` can change under a finished or running review (a selection is edited, the
|
||||
// comparison refreshes), so the result is tied to the items it actually covered.
|
||||
const itemsKey = (ds: WorkspaceItemDiff[]) =>
|
||||
ds
|
||||
.map((d) => `${d.kind}:${d.path}`)
|
||||
.sort()
|
||||
.join('\n')
|
||||
let reviewed = $state<{ key: string; count: number } | undefined>(undefined)
|
||||
let stale = $derived(reviewed != undefined && reviewed.key !== itemsKey(diffs))
|
||||
|
||||
const modelKey = (m: AIProviderModel) => `${m.provider}:${m.model}`
|
||||
// Local to this review: picking a model here must not change the chat's own selection,
|
||||
// which is only the default until something is picked.
|
||||
let pickedModelKey = $state<string | undefined>(undefined)
|
||||
let chatModel = $derived(
|
||||
$copilotSessionModel ?? $copilotInfo.defaultModel ?? $copilotInfo.aiModels[0]
|
||||
)
|
||||
let selectedModelKey = $derived(pickedModelKey ?? (chatModel && modelKey(chatModel)))
|
||||
|
||||
async function run() {
|
||||
abortController?.abort()
|
||||
const controller = new AbortController()
|
||||
abortController = controller
|
||||
running = true
|
||||
error = undefined
|
||||
result = undefined
|
||||
reviewed = { key: itemsKey(diffs), count: diffs.length }
|
||||
logFeatureUsage('deployment_request', 'ai_review', { key: source })
|
||||
try {
|
||||
const review = await reviewDeployment(diffs, forkWorkspaceId, parentWorkspaceId, controller, {
|
||||
model: $copilotInfo.aiModels.find((m) => modelKey(m) === selectedModelKey)
|
||||
})
|
||||
if (!controller.signal.aborted) result = review
|
||||
} catch (e: any) {
|
||||
if (!controller.signal.aborted) error = e?.message ?? String(e)
|
||||
} finally {
|
||||
if (abortController === controller) running = false
|
||||
}
|
||||
}
|
||||
|
||||
$effect(() => () => abortController?.abort())
|
||||
|
||||
function stop() {
|
||||
abortController?.abort()
|
||||
running = false
|
||||
}
|
||||
|
||||
function dismiss(finding: AiReviewFinding) {
|
||||
if (result) result.findings = result.findings.filter((f) => f.id !== finding.id)
|
||||
}
|
||||
|
||||
async function post(finding: AiReviewFinding): Promise<boolean> {
|
||||
if (!onPost) return false
|
||||
postingId = finding.id
|
||||
try {
|
||||
await onPost(finding)
|
||||
logFeatureUsage('deployment_request', 'ai_finding_posted', { key: finding.severity })
|
||||
dismiss(finding)
|
||||
return true
|
||||
} catch (e: any) {
|
||||
sendUserToast(`Failed to post comment: ${e?.body || e?.message}`, true)
|
||||
return false
|
||||
} finally {
|
||||
postingId = undefined
|
||||
}
|
||||
}
|
||||
|
||||
let postingAll = $state(false)
|
||||
|
||||
// One at a time, in the listed order, stopping at the first failure: posted findings leave
|
||||
// the list, so a retry posts only what is left.
|
||||
async function postAll() {
|
||||
postingAll = true
|
||||
try {
|
||||
for (const f of [...(result?.findings ?? [])]) {
|
||||
if (!(await post(f))) break
|
||||
}
|
||||
} finally {
|
||||
postingAll = false
|
||||
}
|
||||
}
|
||||
|
||||
const severityColor = { high: 'red', medium: 'yellow', low: 'gray' } as const
|
||||
|
||||
const SESSION_TARGET_KINDS: string[] = ['script', 'flow', 'raw_app']
|
||||
|
||||
// The session is pinned to the fork: the fixes belong in the workspace under review, and a
|
||||
// session left to pick its own would start in whatever workspace the user last used.
|
||||
function fixSource(findings: AiReviewFinding[]): OpenInSessionSource {
|
||||
const lines = findings.map(
|
||||
(f) => `- [${f.severity}] ${f.kind && f.path ? `${f.kind} ${f.path}: ` : ''}${f.comment}`
|
||||
)
|
||||
const one = findings.length === 1 ? findings[0] : undefined
|
||||
const common = {
|
||||
workspaceId: forkWorkspaceId,
|
||||
seedPrompt: `An AI review of this workspace's changes, before deploying them to ${parentWorkspaceId}, found ${one ? 'this issue' : 'these issues'}. Fix ${one ? 'it' : 'them'} here:\n\n${lines.join('\n')}`,
|
||||
autoSend: true,
|
||||
beforeOpen: () =>
|
||||
logFeatureUsage('deployment_request', 'ai_fix_in_session', {
|
||||
key: one ? 'finding' : 'all'
|
||||
})
|
||||
}
|
||||
if (one && SESSION_TARGET_KINDS.includes(one.kind)) {
|
||||
return { ...common, target: { kind: one.kind as SessionTarget['kind'], path: one.path } }
|
||||
}
|
||||
return {
|
||||
...common,
|
||||
page: () => buildCompareUrl({ workspace_id: forkWorkspaceId, mode: 'fork' })
|
||||
}
|
||||
}
|
||||
</script>
|
||||
|
||||
<div class="flex flex-col gap-2">
|
||||
<div class="flex items-center gap-2">
|
||||
{#if running}
|
||||
<Button variant="default" unifiedSize="sm" startIcon={{ icon: X }} onclick={stop}>
|
||||
Stop review
|
||||
</Button>
|
||||
<span class="flex items-center gap-1 text-xs text-secondary">
|
||||
<Loader2 size={12} class="animate-spin" />
|
||||
Reviewing {reviewed?.count} changed item{reviewed?.count === 1 ? '' : 's'}…
|
||||
</span>
|
||||
{:else}
|
||||
<Button
|
||||
variant="default"
|
||||
unifiedSize="sm"
|
||||
startIcon={{ icon: Sparkles }}
|
||||
disabled={diffs.length === 0}
|
||||
onclick={run}
|
||||
>
|
||||
{result ? 'Review again with AI' : 'Review with AI'}
|
||||
</Button>
|
||||
{#if $copilotInfo.aiModels.length > 1}
|
||||
<div class="w-56">
|
||||
<Select
|
||||
size="sm"
|
||||
items={$copilotInfo.aiModels.map((m) => ({ label: m.model, value: modelKey(m) }))}
|
||||
bind:value={() => selectedModelKey, (v) => (pickedModelKey = v)}
|
||||
/>
|
||||
</div>
|
||||
{/if}
|
||||
{#if diffs.length === 0}
|
||||
<span class="text-xs text-secondary">
|
||||
{source === 'selection' ? 'Select items to review.' : 'No changes to review.'}
|
||||
</span>
|
||||
{/if}
|
||||
{/if}
|
||||
</div>
|
||||
|
||||
{#if error}
|
||||
<Alert type="error" title="AI review failed" size="xs">{error}</Alert>
|
||||
{/if}
|
||||
|
||||
{#if result}
|
||||
<div class="flex flex-col gap-2 border rounded-md p-2 bg-surface-secondary">
|
||||
{#if stale}
|
||||
<Alert type="warning" title="Out of date" size="xs">
|
||||
The items have changed since this review. Review again to cover the current ones.
|
||||
</Alert>
|
||||
{/if}
|
||||
{#if result.summary}
|
||||
<div class="text-xs text-secondary">{result.summary}</div>
|
||||
{/if}
|
||||
{#if result.unreadable.length > 0}
|
||||
<Alert type="warning" title="Not reviewed" size="xs">
|
||||
Could not load the contents of: {result.unreadable.join(', ')}
|
||||
</Alert>
|
||||
{/if}
|
||||
{#if result.omitted.length > 0}
|
||||
<div class="text-2xs text-secondary">
|
||||
Reviewed by name only, too large to include: {result.omitted.join(', ')}
|
||||
</div>
|
||||
{/if}
|
||||
{#if result.findings.length > 1}
|
||||
<div class="flex gap-2 self-start">
|
||||
<OpenInSessionButton
|
||||
source={fixSource(result.findings)}
|
||||
btnClasses={AIBtnClasses('default')}
|
||||
label="Fix all in AI session"
|
||||
tooltip="Open an AI session in the fork and fix every issue listed"
|
||||
btnProps={{ iconOnly: false, unifiedSize: 'xs', startIcon: { icon: WandSparkles } }}
|
||||
/>
|
||||
{#if onPost}
|
||||
<Button
|
||||
variant="default"
|
||||
unifiedSize="xs"
|
||||
startIcon={{ icon: MessageSquarePlus }}
|
||||
disabled={postingId != undefined || postingAll}
|
||||
loading={postingAll}
|
||||
onclick={postAll}
|
||||
>
|
||||
Post all as comments
|
||||
</Button>
|
||||
{/if}
|
||||
</div>
|
||||
{/if}
|
||||
{#if result.findings.length === 0}
|
||||
<div class="text-xs text-secondary">No issues left to review.</div>
|
||||
{:else if source === 'request_form'}
|
||||
<div class="text-2xs text-secondary">
|
||||
Fix these in your fork before sending, or send as is. Reviewers can run their own review.
|
||||
</div>
|
||||
{/if}
|
||||
{#each result.findings as f (f.id)}
|
||||
<div class="border rounded-md p-2 flex flex-col gap-1 bg-surface">
|
||||
<div class="flex items-center gap-2 flex-wrap text-xs">
|
||||
<Badge color={severityColor[f.severity]} size="xs">{f.severity}</Badge>
|
||||
{#if f.kind && f.path}
|
||||
<Badge color="blue" size="xs">{f.kind}:{f.path}</Badge>
|
||||
{/if}
|
||||
</div>
|
||||
<div class="text-xs text-primary whitespace-pre-wrap">{f.comment}</div>
|
||||
<div class="flex gap-2 self-end">
|
||||
<OpenInSessionButton
|
||||
source={fixSource([f])}
|
||||
btnClasses={AIBtnClasses('default')}
|
||||
label="Fix in AI session"
|
||||
tooltip="Open an AI session in the fork and fix this issue"
|
||||
btnProps={{ iconOnly: false, unifiedSize: 'xs', startIcon: { icon: WandSparkles } }}
|
||||
/>
|
||||
{#if onPost}
|
||||
<Button
|
||||
variant="default"
|
||||
unifiedSize="xs"
|
||||
startIcon={{ icon: MessageSquarePlus }}
|
||||
disabled={postingId != undefined || postingAll}
|
||||
loading={postingId === f.id}
|
||||
onclick={() => post(f)}
|
||||
>
|
||||
Post as comment
|
||||
</Button>
|
||||
{/if}
|
||||
<Button variant="subtle" unifiedSize="xs" onclick={() => dismiss(f)}>Dismiss</Button>
|
||||
</div>
|
||||
</div>
|
||||
{/each}
|
||||
</div>
|
||||
{/if}
|
||||
</div>
|
||||
@@ -8,21 +8,33 @@
|
||||
WorkspaceService,
|
||||
type DeploymentRequest,
|
||||
type DeploymentRequestComment,
|
||||
type DeploymentRequestEligibleDeployer
|
||||
type DeploymentRequestEligibleDeployer,
|
||||
type WorkspaceItemDiff
|
||||
} from '$lib/gen'
|
||||
import { sendUserToast } from '$lib/toast'
|
||||
import { copilotInfo } from '$lib/aiStore'
|
||||
import { Send, Plus, X, ArchiveX } from 'lucide-svelte'
|
||||
import AiReview from './AiReview.svelte'
|
||||
import type { AiReviewFinding } from './aiReview'
|
||||
|
||||
interface Props {
|
||||
forkWorkspaceId: string
|
||||
parentWorkspaceId: string
|
||||
currentUsername: string
|
||||
isAdmin: boolean
|
||||
/** The changes the request would merge into the parent, for the AI review. */
|
||||
diffs: WorkspaceItemDiff[]
|
||||
onStateChange?: (hasOpenRequest: boolean) => void
|
||||
}
|
||||
|
||||
let { forkWorkspaceId, parentWorkspaceId, currentUsername, isAdmin, onStateChange }: Props =
|
||||
$props()
|
||||
let {
|
||||
forkWorkspaceId,
|
||||
parentWorkspaceId,
|
||||
currentUsername,
|
||||
isAdmin,
|
||||
diffs,
|
||||
onStateChange
|
||||
}: Props = $props()
|
||||
|
||||
let request = $state<DeploymentRequest | null>(null)
|
||||
let loading = $state(true)
|
||||
@@ -114,6 +126,20 @@
|
||||
}
|
||||
}
|
||||
|
||||
async function postAiFinding(finding: AiReviewFinding): Promise<void> {
|
||||
if (!request) return
|
||||
const created = await WorkspaceService.createDeploymentRequestComment({
|
||||
workspace: forkWorkspaceId,
|
||||
id: request.id,
|
||||
requestBody: {
|
||||
body: finding.comment,
|
||||
anchor_kind: finding.kind || undefined,
|
||||
anchor_path: finding.path || undefined
|
||||
}
|
||||
})
|
||||
request = { ...request, comments: [...request.comments, created] }
|
||||
}
|
||||
|
||||
async function postComment(body: string, parentId?: number) {
|
||||
if (!request || !body.trim()) return
|
||||
posting = true
|
||||
@@ -233,6 +259,11 @@
|
||||
Cancel
|
||||
</Button>
|
||||
</div>
|
||||
{#if $copilotInfo.enabled}
|
||||
<div class="border-t pt-2">
|
||||
<AiReview {diffs} {forkWorkspaceId} {parentWorkspaceId} source="request_form" />
|
||||
</div>
|
||||
{/if}
|
||||
</div>
|
||||
{:else}
|
||||
<div class="flex items-center justify-between flex-wrap gap-2 text-xs">
|
||||
@@ -261,6 +292,15 @@
|
||||
</div>
|
||||
|
||||
<div class="border rounded-md bg-surface p-3 flex flex-col gap-3">
|
||||
{#if $copilotInfo.enabled}
|
||||
<AiReview
|
||||
{diffs}
|
||||
{forkWorkspaceId}
|
||||
{parentWorkspaceId}
|
||||
source="open_request"
|
||||
onPost={postAiFinding}
|
||||
/>
|
||||
{/if}
|
||||
{#if topLevel.length === 0}
|
||||
<div class="text-secondary text-xs">No comments yet. Be the first to leave feedback.</div>
|
||||
{/if}
|
||||
|
||||
@@ -0,0 +1,84 @@
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import type { WorkspaceItemDiff } from '$lib/gen'
|
||||
|
||||
const h = vi.hoisted(() => ({ prompt: '', workspace: '' as string | undefined, reads: 0 }))
|
||||
|
||||
vi.mock('$lib/components/copilot/lib', () => ({
|
||||
getNonStreamingCompletion: async (
|
||||
messages: { content: string }[],
|
||||
_abort: AbortController,
|
||||
options?: { workspace?: string }
|
||||
) => {
|
||||
h.prompt = messages[1].content
|
||||
h.workspace = options?.workspace
|
||||
return '{"summary":"s","findings":[]}'
|
||||
}
|
||||
}))
|
||||
// Mirrors the shared loader, which answers `{}` instead of throwing when a read fails.
|
||||
vi.mock('$lib/utils_workspace_deploy', () => ({
|
||||
getItemValue: async (_kind: string, path: string) => {
|
||||
h.reads++
|
||||
if (path === 'f/a/unreadable') return {}
|
||||
return { content: path.startsWith('f/big/') ? 'x'.repeat(30_000) : 'x' }
|
||||
}
|
||||
}))
|
||||
|
||||
const { parseReviewAnswer, reviewDeployment } = await import('./aiReview')
|
||||
|
||||
function diff(path: string): WorkspaceItemDiff {
|
||||
return {
|
||||
kind: 'script',
|
||||
path,
|
||||
ahead: 1,
|
||||
behind: 0,
|
||||
has_changes: true,
|
||||
exists_in_source: true,
|
||||
exists_in_fork: true
|
||||
}
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
h.reads = 0
|
||||
})
|
||||
|
||||
describe('parseReviewAnswer', () => {
|
||||
it('anchors only findings on items in the diff, and ranks by severity', () => {
|
||||
const answer =
|
||||
'```json\n' +
|
||||
JSON.stringify({
|
||||
summary: 's',
|
||||
findings: [
|
||||
{ kind: 'flow', path: 'f/not/changed', severity: 'low', comment: 'phantom' },
|
||||
{ kind: 'script', path: 'f/a/b', severity: 'high', comment: 'real' }
|
||||
]
|
||||
}) +
|
||||
'\n```'
|
||||
const { findings } = parseReviewAnswer(answer, [diff('f/a/b')])
|
||||
expect(findings.map((f) => [f.comment, f.kind, f.path])).toEqual([
|
||||
['real', 'script', 'f/a/b'],
|
||||
['phantom', '', '']
|
||||
])
|
||||
})
|
||||
})
|
||||
|
||||
describe('reviewDeployment', () => {
|
||||
it('reports an item whose read failed instead of reviewing it as emptied', async () => {
|
||||
const result = await reviewDeployment(
|
||||
[diff('f/a/b'), diff('f/a/unreadable')],
|
||||
'fork',
|
||||
'parent',
|
||||
new AbortController()
|
||||
)
|
||||
expect(result.unreadable).toEqual(['script:f/a/unreadable'])
|
||||
expect(h.prompt).toContain('### script f/a/unreadable (modified)\nContents could not be read.')
|
||||
expect(h.workspace).toBe('fork')
|
||||
})
|
||||
|
||||
it('stops reading items once a section no longer fits the budget', async () => {
|
||||
const diffs = Array.from({ length: 30 }, (_, i) => diff(`f/big/${i}`))
|
||||
const result = await reviewDeployment(diffs, 'fork', 'parent', new AbortController())
|
||||
// Only the first batch of 6 is read, both sides each.
|
||||
expect(h.reads).toBe(12)
|
||||
expect(result.omitted).toContain('script:f/big/29')
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,188 @@
|
||||
import type { AIProviderModel, WorkspaceItemDiff } from '$lib/gen'
|
||||
import { getNonStreamingCompletion } from '$lib/components/copilot/lib'
|
||||
import { getItemValue } from '$lib/utils_workspace_deploy'
|
||||
import type { Kind } from '$lib/utils_deployable'
|
||||
|
||||
export type AiReviewSeverity = 'high' | 'medium' | 'low'
|
||||
|
||||
export type AiReviewFinding = {
|
||||
id: string
|
||||
kind: string
|
||||
path: string
|
||||
severity: AiReviewSeverity
|
||||
comment: string
|
||||
}
|
||||
|
||||
export type AiReviewResult = {
|
||||
summary: string
|
||||
findings: AiReviewFinding[]
|
||||
/** Changed items whose contents did not fit in the prompt and were reviewed by name only. */
|
||||
omitted: string[]
|
||||
/** Changed items whose contents failed to load, so the model did not review them. */
|
||||
unreadable: string[]
|
||||
}
|
||||
|
||||
// A variable's value is read decrypted and a resource's value holds credentials, so neither may
|
||||
// reach the model: they are reviewed by name and change type only.
|
||||
const VALUE_WITHHELD_KINDS = new Set(['variable', 'resource'])
|
||||
|
||||
const PER_SIDE_CHAR_CAP = 20_000
|
||||
const TOTAL_CHAR_BUDGET = 160_000
|
||||
const FETCH_BATCH = 6
|
||||
// Under the Anthropic SDK's non-streaming pre-flight max_tokens limit.
|
||||
const MAX_OUTPUT_TOKENS = 8_000
|
||||
|
||||
const SYSTEM_PROMPT = `You review a Windmill deployment request: a set of changes made in a fork workspace that will be merged into its parent workspace. Each changed item is given with its parent version (what is deployed now) and its fork version (what will be deployed). An item can be a script, flow, app, schedule, trigger, folder, resource type, datatable migration, variable or resource.
|
||||
|
||||
Report only issues a reviewer should act on before approving the merge:
|
||||
- bugs and logic errors introduced by the change
|
||||
- breaking changes: removed or renamed inputs, changed input types, changed outputs that callers may rely on
|
||||
- risky operational changes: schedules retimed, triggers pointed at a different script or flow, folder owners or permissions, app policies, destructive migrations
|
||||
- hardcoded secrets or credentials
|
||||
- deletions that look unintended
|
||||
|
||||
Do not comment on style, naming, formatting or lockfiles, and do not praise. Do not repeat a finding for every item it touches. An item with no issue gets no finding.
|
||||
|
||||
Answer with JSON only, no prose around it, in this shape:
|
||||
{"summary": "<one or two sentences on what the change does and its overall risk>", "findings": [{"kind": "<item kind as given>", "path": "<item path as given>", "severity": "high" | "medium" | "low", "comment": "<the issue and what to do about it, under 80 words>"}]}`
|
||||
|
||||
function changeType(diff: WorkspaceItemDiff): 'added' | 'deleted' | 'modified' {
|
||||
if (!diff.exists_in_source) return 'added'
|
||||
if (!diff.exists_in_fork) return 'deleted'
|
||||
return 'modified'
|
||||
}
|
||||
|
||||
function render(value: unknown): string {
|
||||
if (value == null) return '(none)'
|
||||
// Flows and apps nest their inline scripts' lockfiles, so they are dropped at any depth.
|
||||
const text =
|
||||
typeof value === 'string'
|
||||
? value
|
||||
: JSON.stringify(value, (key, v) => (key === 'lock' ? undefined : v), 2)
|
||||
return text.length > PER_SIDE_CHAR_CAP
|
||||
? `${text.slice(0, PER_SIDE_CHAR_CAP)}\n… (truncated, ${text.length - PER_SIDE_CHAR_CAP} more characters)`
|
||||
: text
|
||||
}
|
||||
|
||||
async function itemSection(
|
||||
diff: WorkspaceItemDiff,
|
||||
forkWorkspaceId: string,
|
||||
parentWorkspaceId: string
|
||||
): Promise<{ text: string; unreadable: boolean }> {
|
||||
const change = changeType(diff)
|
||||
const header = `### ${diff.kind} ${diff.path} (${change})`
|
||||
if (VALUE_WITHHELD_KINDS.has(diff.kind)) {
|
||||
return { text: `${header}\nValue withheld from review.`, unreadable: false }
|
||||
}
|
||||
const kind = diff.kind as Kind
|
||||
const [parent, fork] = await Promise.all([
|
||||
change === 'added' ? undefined : getItemValue(kind, diff.path, parentWorkspaceId),
|
||||
change === 'deleted' ? undefined : getItemValue(kind, diff.path, forkWorkspaceId)
|
||||
])
|
||||
// `getItemValue` answers `{}` when a read fails, which would pass as an emptied item.
|
||||
if (isEmptyRead(parent) || isEmptyRead(fork)) {
|
||||
return { text: `${header}\nContents could not be read.`, unreadable: true }
|
||||
}
|
||||
return {
|
||||
text: `${header}\n#### Parent version\n${render(parent)}\n#### Fork version\n${render(fork)}`,
|
||||
unreadable: false
|
||||
}
|
||||
}
|
||||
|
||||
function isEmptyRead(value: unknown): boolean {
|
||||
return value != null && typeof value === 'object' && Object.keys(value as object).length === 0
|
||||
}
|
||||
|
||||
export function parseReviewAnswer(
|
||||
text: string,
|
||||
diffs: WorkspaceItemDiff[]
|
||||
): Omit<AiReviewResult, 'omitted' | 'unreadable'> {
|
||||
const start = text.indexOf('{')
|
||||
const end = text.lastIndexOf('}')
|
||||
if (start === -1 || end <= start) {
|
||||
throw new Error('The AI answer was not in the expected format')
|
||||
}
|
||||
const parsed = JSON.parse(text.slice(start, end + 1)) as {
|
||||
summary?: unknown
|
||||
findings?: unknown
|
||||
}
|
||||
// Anchoring a comment needs a (kind, path) the fork's diff actually has, so a finding naming
|
||||
// anything else is kept but unanchored rather than posted against a phantom item.
|
||||
const known = new Set(diffs.map((d) => `${d.kind}:${d.path}`))
|
||||
const findings: AiReviewFinding[] = []
|
||||
for (const [i, raw] of (Array.isArray(parsed.findings) ? parsed.findings : []).entries()) {
|
||||
if (!raw || typeof raw.comment !== 'string' || !raw.comment.trim()) continue
|
||||
const anchored = known.has(`${raw.kind}:${raw.path}`)
|
||||
findings.push({
|
||||
id: String(i),
|
||||
kind: anchored ? raw.kind : '',
|
||||
path: anchored ? raw.path : '',
|
||||
severity: ['high', 'medium', 'low'].includes(raw.severity) ? raw.severity : 'medium',
|
||||
comment: raw.comment.trim()
|
||||
})
|
||||
}
|
||||
const rank: Record<AiReviewSeverity, number> = { high: 0, medium: 1, low: 2 }
|
||||
findings.sort((a, b) => rank[a.severity] - rank[b.severity])
|
||||
return { summary: typeof parsed.summary === 'string' ? parsed.summary : '', findings }
|
||||
}
|
||||
|
||||
/** Reviews the changes `diffs` carry from the fork into its parent with the workspace's AI model. */
|
||||
export async function reviewDeployment(
|
||||
diffs: WorkspaceItemDiff[],
|
||||
forkWorkspaceId: string,
|
||||
parentWorkspaceId: string,
|
||||
abortController: AbortController,
|
||||
options?: {
|
||||
/** Defaults to the chat's current model. */
|
||||
model?: AIProviderModel
|
||||
}
|
||||
): Promise<AiReviewResult> {
|
||||
const included: string[] = []
|
||||
const omitted: string[] = []
|
||||
const unreadable: string[] = []
|
||||
let used = 0
|
||||
// Fetched a batch at a time so a large fork stops reading items once the budget is spent.
|
||||
// The first section that does not fit spends it: `used` alone can stay under the cap forever.
|
||||
let budgetSpent = false
|
||||
for (let i = 0; i < diffs.length; i += FETCH_BATCH) {
|
||||
const batch = diffs.slice(i, i + FETCH_BATCH)
|
||||
if (budgetSpent) {
|
||||
omitted.push(...batch.map((d) => `${d.kind}:${d.path}`))
|
||||
continue
|
||||
}
|
||||
const sections = await Promise.all(
|
||||
batch.map((d) => itemSection(d, forkWorkspaceId, parentWorkspaceId))
|
||||
)
|
||||
if (abortController.signal.aborted) throw new Error('aborted')
|
||||
sections.forEach(({ text: section, unreadable: failed }, j) => {
|
||||
if (failed) unreadable.push(`${batch[j].kind}:${batch[j].path}`)
|
||||
if (!budgetSpent && used + section.length <= TOTAL_CHAR_BUDGET) {
|
||||
included.push(section)
|
||||
used += section.length
|
||||
} else {
|
||||
budgetSpent = true
|
||||
omitted.push(`${batch[j].kind}:${batch[j].path}`)
|
||||
}
|
||||
})
|
||||
}
|
||||
let user = `Fork workspace: ${forkWorkspaceId}\nParent workspace: ${parentWorkspaceId}\n\n${included.join('\n\n')}`
|
||||
if (omitted.length > 0) {
|
||||
user += `\n\nAlso changed, contents not shown for size:\n${omitted.map((o) => `- ${o}`).join('\n')}`
|
||||
}
|
||||
|
||||
const answer = await getNonStreamingCompletion(
|
||||
[
|
||||
{ role: 'system', content: SYSTEM_PROMPT },
|
||||
{ role: 'user', content: user }
|
||||
],
|
||||
abortController,
|
||||
// Bound to the fork: without `workspace` the call goes through whichever workspace the
|
||||
// app has switched to by the time the item reads finish.
|
||||
{
|
||||
maxTokensCap: MAX_OUTPUT_TOKENS,
|
||||
forceModelProvider: options?.model,
|
||||
workspace: forkWorkspaceId
|
||||
}
|
||||
)
|
||||
return { ...parseReviewAnswer(answer ?? '', diffs), omitted, unreadable }
|
||||
}
|
||||
Reference in New Issue
Block a user