feat: add ai review to drafts, draft diffs and session review (#12063)

* feat: add ai review to the drafts page, draft diffs and the session review

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix: drop an ai review when its workspace changes

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix: follow the diff version picker in ai review and review only your own draft from the badge

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix: give the drafts page ai review the full width

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Ruben Fiszel
2026-10-28 17:28:41 +01:00
committed by GitHub
co-authored by Claude Opus 5.5
parent 499c7569ff
commit 77c0a26515
18 changed files with 476 additions and 127 deletions
+21 -6
View File
@@ -74,7 +74,7 @@ export function formatFolderInstructions(
}
// What the review asks of the model and how its answer is read, for every place it runs: the
// deployment request panel here, and `wmill review` on a git sync pull request, which carries
// deployment request panel and the drafts review here, and `wmill review` on a git sync pull request, which carries
// this module through cli/generate-ai-review.ts. Keep it free of `$lib` imports; the
// staleness test in cli/test/ai_review_bundle_unit.test.ts fails when the CLI's copy drifts.
@@ -109,32 +109,47 @@ export const TOTAL_CHAR_BUDGET = 160_000
// Under the Anthropic SDK's non-streaming pre-flight max_tokens limit.
export const MAX_OUTPUT_TOKENS = 8_000
/** What is being reviewed: a fork's deployment request, or a pull request on a git sync repo. */
export type ReviewSubject = 'deployment_request' | 'pull_request'
/**
* What is being reviewed: a fork's deployment request, a workspace's undeployed drafts, or a
* pull request on a git sync repo.
*/
export type ReviewSubject = 'deployment_request' | 'draft' | 'pull_request'
const INTRO: Record<ReviewSubject, string> = {
deployment_request:
'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).',
draft:
'You review drafts in a Windmill workspace: changes saved to items of the workspace but not deployed yet. Each changed item is given with its deployed version (what is deployed now) and its draft version (what will be deployed).',
pull_request:
"You review a pull request on a git repository that Windmill's git sync keeps in step with a workspace: merging it deploys its changes. Each changed item is given with the files that make it up in the repository, each as a unified diff of the base branch (what is deployed now) against the pull request (what will be deployed). Every diff line starts with its line number: R<n> for line n of the pull request's version of the file, L<n> for line n of the base version, then + for an added line, - for a removed one and a space for unchanged context."
}
const ITEM_ANSWER_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>"}]}'
const ANSWER_SHAPE: Record<ReviewSubject, string> = {
deployment_request:
'{"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>"}]}',
deployment_request: ITEM_ANSWER_SHAPE,
draft: ITEM_ANSWER_SHAPE,
pull_request:
'{"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>", "file": "<the file the issue is in, as given>", "line": "<the R or L line number the issue is on, as given, such as R12, or null for the file as a whole>", "severity": "high" | "medium" | "low", "comment": "<the issue and what to do about it, under 80 words>"}]}'
}
const SOURCE: Record<ReviewSubject, string> = {
deployment_request: 'The parent workspace',
draft: 'The workspace',
pull_request: "The repository's base branch"
}
const APPROVED: Record<ReviewSubject, string> = {
deployment_request: 'the merge',
draft: 'the deployment',
pull_request: 'the merge'
}
export function reviewSystemPrompt(subject: ReviewSubject): string {
return `${INTRO[subject]} 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:
Report only issues a reviewer should act on before approving ${APPROVED[subject]}:
- 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
@@ -42,6 +42,9 @@
import { userStore, userWorkspaces } from '$lib/stores'
import { capabilitiesIn } from '$lib/roleCapabilities'
import { childWorkspaceNoun } from '$lib/utils/devWorkspaceLabel'
import { copilotInfo } from '$lib/aiStore'
import AiReview from './deploymentRequest/AiReview.svelte'
import { draftReviewItem } from './deploymentRequest/draftReviewItem'
interface Props {
currentWorkspaceId: string
@@ -456,6 +459,13 @@
selectedItems = []
}
// Your own drafts only: another user's draft is view-only, and reading one returns yours.
const reviewItems = $derived(
visibleItems
.filter((i) => selectedItems.includes(i.key) && i.mine && i.draftKind !== 'data_pipeline')
.map((i) => draftReviewItem(i.draftKind, i.path, currentWorkspaceId, i.draft_only))
)
// --- Diff ---
let diffDrawer: DiffDrawer | undefined = $state(undefined)
let isFlow = $state(false)
@@ -480,7 +490,13 @@
mode: 'simple',
original: deployed as any,
current: draft as any,
title: 'Deployed → Draft'
title: 'Deployed → Draft',
aiReview: {
workspaceId: currentWorkspaceId,
kind: item.draftKind,
path: item.path,
added: item.draft_only
}
})
}
@@ -912,35 +928,45 @@
{/snippet}
{#snippet footer()}
<div class="flex flex-col items-end gap-2">
<div class="flex items-center gap-2">
<Button
variant="default"
destructive
disabled={discardableCount === 0 || deploying || discarding}
loading={discarding}
startIcon={{ icon: Undo2 }}
onClick={onDiscardSelectedClick}
>
Discard {discardableCount} draft{discardableCount !== 1 ? 's' : ''}
</Button>
<Button
variant="accent"
disabled={deployableCount === 0 || deploying || discarding || !deployPerm.ok}
title={!deployPerm.ok ? deployPerm.reason : undefined}
loading={deploying}
onClick={deploySelected}
>
Deploy {deployableCount} draft{deployableCount !== 1 ? 's' : ''}
</Button>
<div class="flex flex-col gap-4">
<div class="flex flex-col items-end gap-2">
<div class="flex items-center gap-2">
<Button
variant="default"
destructive
disabled={discardableCount === 0 || deploying || discarding}
loading={discarding}
startIcon={{ icon: Undo2 }}
onClick={onDiscardSelectedClick}
>
Discard {discardableCount} draft{discardableCount !== 1 ? 's' : ''}
</Button>
<Button
variant="accent"
disabled={deployableCount === 0 || deploying || discarding || !deployPerm.ok}
title={!deployPerm.ok ? deployPerm.reason : undefined}
loading={deploying}
onClick={deploySelected}
>
Deploy {deployableCount} draft{deployableCount !== 1 ? 's' : ''}
</Button>
</div>
{#if !deployPerm.ok}
<span class="text-xs text-yellow-600">{deployPerm.reason}</span>
{:else if undeployableSelectedCount > 0}
<span class="text-xs text-secondary">
{undeployableSelectedCount} selected draft{undeployableSelectedCount !== 1
? 's'
: ''}
can't be deployed (no write permission on the path) but can still be discarded
</span>
{/if}
</div>
{#if !deployPerm.ok}
<span class="text-xs text-yellow-600">{deployPerm.reason}</span>
{:else if undeployableSelectedCount > 0}
<span class="text-xs text-secondary">
{undeployableSelectedCount} selected draft{undeployableSelectedCount !== 1 ? 's' : ''}
can't be deployed (no write permission on the path) but can still be discarded
</span>
{#if $copilotInfo.enabled}
<AiReview
scope={{ kind: 'drafts', workspaceId: currentWorkspaceId, items: reviewItems }}
source="drafts"
/>
{/if}
</div>
{/snippet}
@@ -1854,9 +1854,12 @@
<!-- The request form and an open request carry their own review. -->
{#if mergeIntoParent && $copilotInfo.enabled && !hasOpenDeploymentRequest && !deploymentRequestPanel?.isDialogOpen()}
<AiReview
diffs={selectedDiffs}
forkWorkspaceId={currentWorkspaceId}
{parentWorkspaceId}
scope={{
kind: 'deployment',
diffs: selectedDiffs,
forkWorkspaceId: currentWorkspaceId,
parentWorkspaceId
}}
source="selection"
/>
{/if}
+52 -1
View File
@@ -14,7 +14,10 @@
import { sendUserToast } from '$lib/toast'
import type { Script } from '$lib/gen'
import Select from './select/Select.svelte'
import type { DiffVersionOption } from './diff_drawer'
import type { DiffAiReview, DiffVersionOption } from './diff_drawer'
import { copilotInfo } from '$lib/aiStore'
import AiReview from './deploymentRequest/AiReview.svelte'
import type { ReviewScope } from './deploymentRequest/aiReview'
type DiffData = {
lang?: string
@@ -36,6 +39,25 @@
let diffViewer: Drawer | undefined = $state(undefined)
let aiReviewScope: ReviewScope | undefined = $state(undefined)
/** Normal mode: what the review is rebuilt from when the version picker swaps the left side. */
let aiReviewTarget: { review: DiffAiReview; current: Value } | undefined = undefined
function reviewScope(review: DiffAiReview, before: Value, after: Value): ReviewScope {
return {
kind: 'drafts',
workspaceId: review.workspaceId,
items: [
{
kind: review.kind,
path: review.path,
change: review.added ? 'added' : 'modified',
load: async () => ({ before, after })
}
]
}
}
interface Props {
restoreDeployed?: () => Promise<void>
isFlow?: boolean
@@ -77,6 +99,8 @@
if (token == null) openingToken++
data = undefined
diffType = undefined
aiReviewScope = undefined
aiReviewTarget = undefined
diffViewer?.openDrawer()
}
@@ -111,6 +135,8 @@
loadingVersion = false
data = undefined
diffType = undefined
aiReviewScope = undefined
aiReviewTarget = undefined
diffViewer?.closeDrawer()
}
@@ -209,6 +235,11 @@
deployed: prepareDiff(value),
deployedLabel: opt?.isHead ? headLabel : opt?.label
}
// The review covers the version on screen; a new scope also drops a result for the
// previous one.
if (aiReviewTarget) {
aiReviewScope = reviewScope(aiReviewTarget.review, value, aiReviewTarget.current)
}
} catch (e: any) {
// The snap-back would otherwise be the only sign the version never loaded.
if (generation === versionLoadGeneration) {
@@ -270,6 +301,7 @@
current: Value
defaultDiffType?: 'deployed' | 'draft'
button?: { text: string; onClick: () => void }
aiReview?: DiffAiReview
}
| {
mode: 'simple'
@@ -277,6 +309,7 @@
current: Value
title: string
button?: { text: string; onClick: () => void }
aiReview?: DiffAiReview
},
token?: number
) {
@@ -284,6 +317,8 @@
// caller with none is taking the drawer, so it claims one.
if (token != null && token !== openingToken) return
if (token == null) openingToken++
aiReviewScope = undefined
aiReviewTarget = undefined
if (diff.mode === 'normal') {
const {
deployed,
@@ -326,6 +361,13 @@
// The draft-vs-current view (and its tab) is obsolete — always show the
// deployed-vs-current diff.
diffType = 'deployed'
if (diff.aiReview) {
aiReviewTarget = {
review: { ...diff.aiReview, added: diff.aiReview.added || !!deployed.draft_only },
current
}
aiReviewScope = reviewScope(aiReviewTarget.review, deployed, current)
}
} else {
const { original, current, title, button } = diff
data = {
@@ -336,6 +378,7 @@
button
}
diffType = 'custom'
if (diff.aiReview) aiReviewScope = reviewScope(diff.aiReview, original, current)
}
}
</script>
@@ -343,6 +386,14 @@
<Drawer bind:this={diffViewer} size="1200px" on:close>
<DrawerContent title="Diff" on:close={diffViewer.closeDrawer}>
<div class="flex flex-col gap-4 h-full">
{#if data && aiReviewScope && $copilotInfo.enabled}
<div class="max-h-[40vh] overflow-y-auto shrink-0">
<!-- A new diff is a new review: a result never carries over to it. -->
{#key aiReviewScope}
<AiReview scope={aiReviewScope} source="draft_diff" />
{/key}
</div>
{/if}
{#if data}
<!-- Outside the `contentType` check on purpose: with no differences against
the head, picking an older version is exactly how the reader finds one,
@@ -178,7 +178,12 @@
mode: 'simple',
title: `${fullLabel(owner)}'s draft vs deployed`,
original: deployed,
current: draftValue as any
current: draftValue as any,
// Your own draft only: a fix made from the review edits your draft, not theirs.
aiReview:
currentUsername && owner.username === currentUsername
? { workspaceId: workspace, kind: itemKind, path, added: draft_only }
: undefined
})
} catch (e: any) {
sendUserToast(`Could not load draft: ${e.body ?? e.message}`, true)
@@ -1357,7 +1357,10 @@
...currentFlow,
path: $pathStore,
draft_triggers: currentDraftTriggers
}
},
aiReview: opWorkspace
? { workspaceId: opWorkspace, kind: 'flow', path: $pathStore }
: undefined
},
opening
)
@@ -1156,7 +1156,10 @@
})
},
draft: savedScript['draft'],
current
current,
aiReview: opWorkspace
? { workspaceId: opWorkspace, kind: 'script', path: script.path }
: undefined
},
opening
)
@@ -758,7 +758,10 @@
policy,
custom_path: customPath,
labels
}
},
aiReview: $workspaceStore
? { workspaceId: $workspaceStore, kind: 'app', path: savedApp.path }
: undefined
},
opening
)
@@ -904,6 +907,9 @@
custom_path: customPath,
labels
},
aiReview: $workspaceStore
? { workspaceId: $workspaceStore, kind: 'app', path: savedApp.path }
: undefined,
button: {
text: 'Looks good, deploy',
onClick: () => {
@@ -1,6 +1,6 @@
<script lang="ts">
import { Alert, Badge, Button } from '$lib/components/common'
import type { AIProviderModel, WorkspaceItemDiff } from '$lib/gen'
import type { AIProviderModel } from '$lib/gen'
import { copilotInfo, copilotSessionModel } from '$lib/aiStore'
import Select from '$lib/components/select/Select.svelte'
import { sendUserToast } from '$lib/toast'
@@ -12,19 +12,38 @@
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'
import {
instructionsWorkspace,
reviewChanges,
scopeItems,
scopeWorkspace,
type AiReviewFinding,
type AiReviewResult,
type ReviewScope
} 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'
scope: ReviewScope
/**
* Where the review runs: the items selected for deploy, the request form, an open request,
* the drafts selected on the drafts page, one draft's diff, or a session's edits.
*/
source:
| 'selection'
| 'request_form'
| 'open_request'
| 'drafts'
| 'draft_diff'
| 'session_edits'
/** 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 { scope, source, onPost }: Props = $props()
let items = $derived(scopeItems(scope))
let workspaceId = $derived(scopeWorkspace(scope))
let instructionsWorkspaceId = $derived(instructionsWorkspace(scope))
let running = $state(false)
let result = $state<AiReviewResult | undefined>(undefined)
@@ -32,15 +51,15 @@
let postingId = $state<string | undefined>(undefined)
let abortController: AbortController | undefined
// `diffs` can change under a finished or running review (a selection is edited, the
// The items 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[]) =>
const itemsKey = (ds: { kind: string; path: string }[]) =>
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))
let stale = $derived(reviewed != undefined && reviewed.key !== itemsKey(items))
const modelKey = (m: AIProviderModel) => `${m.provider}:${m.model}`
// Local to this review: picking a model here must not change the chat's own selection,
@@ -58,10 +77,10 @@
running = true
error = undefined
result = undefined
reviewed = { key: itemsKey(diffs), count: diffs.length }
reviewed = { key: itemsKey(items), count: items.length }
logFeatureUsage('deployment_request', 'ai_review', { key: source })
try {
const review = await reviewDeployment(diffs, forkWorkspaceId, parentWorkspaceId, controller, {
const review = await reviewChanges(scope, controller, {
model: $copilotInfo.aiModels.find((m) => modelKey(m) === selectedModelKey)
})
if (!controller.signal.aborted) result = review
@@ -74,6 +93,21 @@
$effect(() => () => abortController?.abort())
// A review answers for the workspaces it ran in. The same paths in another workspace are
// other items, so a switch drops the result and any review still running, rather than
// leaving it to read as current there.
$effect(() => {
void workspaceId
void instructionsWorkspaceId
return () => {
abortController?.abort()
running = false
result = undefined
error = undefined
reviewed = undefined
}
})
function stop() {
abortController?.abort()
running = false
@@ -118,16 +152,20 @@
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.
// The session is pinned to the reviewed workspace: the fixes belong there, 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')}`,
workspaceId,
seedPrompt: `${
scope.kind === 'deployment'
? `An AI review of this workspace's changes, before deploying them to ${scope.parentWorkspaceId}`
: "An AI review of this workspace's drafts, before deploying them"
}, 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', {
@@ -139,7 +177,11 @@
}
return {
...common,
page: () => buildCompareUrl({ workspace_id: forkWorkspaceId, mode: 'fork' })
page: () =>
buildCompareUrl({
workspace_id: workspaceId,
mode: scope.kind === 'deployment' ? 'fork' : 'draft'
})
}
}
</script>
@@ -159,7 +201,7 @@
variant="default"
unifiedSize="sm"
startIcon={{ icon: Sparkles }}
disabled={diffs.length === 0}
disabled={items.length === 0}
onclick={run}
>
{result ? 'Review again with AI' : 'Review with AI'}
@@ -173,9 +215,11 @@
/>
</div>
{/if}
{#if diffs.length === 0}
{#if items.length === 0}
<span class="text-xs text-secondary">
{source === 'selection' ? 'Select items to review.' : 'No changes to review.'}
{source === 'selection' || source === 'drafts'
? 'Select items to review.'
: 'No changes to review.'}
</span>
{/if}
{/if}
@@ -199,15 +243,15 @@
{#if result.instructions.length > 0}
Applied {result.instructions.length} review instruction{result.instructions.length === 1
? ''
: 's'} from {parentWorkspaceId}: {result.instructions.join(', ')}.
: 's'} from {instructionsWorkspaceId}: {result.instructions.join(', ')}.
{:else}
No review instructions from {parentWorkspaceId} applied.
No review instructions from {instructionsWorkspaceId} applied.
{/if}
Only the ai_review_instruction resources you can read in {parentWorkspaceId} are used.
Only the ai_review_instruction resources you can read in {instructionsWorkspaceId} are used.
</div>
{#if result.instructionsUnavailable}
<Alert type="warning" title="Review instructions unavailable" size="xs">
Could not list the review instructions of {parentWorkspaceId}, so none were applied.
Could not list the review instructions of {instructionsWorkspaceId}, so none were applied.
</Alert>
{/if}
{#if result.skippedInstructions.length > 0}
@@ -231,7 +275,7 @@
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"
tooltip="Open an AI session in {workspaceId} and fix every issue listed"
btnProps={{ iconOnly: false, unifiedSize: 'xs', startIcon: { icon: WandSparkles } }}
/>
{#if onPost}
@@ -269,7 +313,7 @@
source={fixSource([f])}
btnClasses={AIBtnClasses('default')}
label="Fix in AI session"
tooltip="Open an AI session in the fork and fix this issue"
tooltip="Open an AI session in {workspaceId} and fix this issue"
btnProps={{ iconOnly: false, unifiedSize: 'xs', startIcon: { icon: WandSparkles } }}
/>
{#if onPost}
@@ -261,7 +261,10 @@
</div>
{#if $copilotInfo.enabled}
<div class="border-t pt-2">
<AiReview {diffs} {forkWorkspaceId} {parentWorkspaceId} source="request_form" />
<AiReview
scope={{ kind: 'deployment', diffs, forkWorkspaceId, parentWorkspaceId }}
source="request_form"
/>
</div>
{/if}
</div>
@@ -294,9 +297,7 @@
<div class="border rounded-md bg-surface p-3 flex flex-col gap-3">
{#if $copilotInfo.enabled}
<AiReview
{diffs}
{forkWorkspaceId}
{parentWorkspaceId}
scope={{ kind: 'deployment', diffs, forkWorkspaceId, parentWorkspaceId }}
source="open_request"
onPost={postAiFinding}
/>
@@ -49,7 +49,15 @@ vi.mock('$lib/gen', async (importOriginal) => ({
}
}))
const { reviewDeployment } = await import('./aiReview')
const { reviewChanges } = await import('./aiReview')
const reviewDeployment = (
diffs: WorkspaceItemDiff[],
forkWorkspaceId: string,
parentWorkspaceId: string,
abortController: AbortController
) =>
reviewChanges({ kind: 'deployment', diffs, forkWorkspaceId, parentWorkspaceId }, abortController)
function diff(path: string): WorkspaceItemDiff {
return {
@@ -107,3 +115,39 @@ describe('reviewDeployment', () => {
expect(result.instructions).toEqual([])
})
})
describe('drafts review', () => {
it("reviews drafts as deployed against draft, under their own workspace's instructions", async () => {
const result = await reviewChanges(
{
kind: 'drafts',
workspaceId: 'parent',
items: [
{
kind: 'script',
path: 'f/a/b',
change: 'modified',
load: async () => ({ before: 'old body', after: 'new body' })
},
{
kind: 'flow',
path: 'f/a/c',
change: 'added',
load: async () => {
throw new Error('404')
}
}
]
},
new AbortController()
)
expect(h.prompt).toContain(
'### script f/a/b (modified)\n#### Deployed version\nold body\n#### Draft version\nnew body'
)
expect(result.unreadable).toEqual(['flow:f/a/c'])
expect(result.instructions).toEqual(['f/a/REVIEW'])
expect(h.listed).toEqual(['parent'])
expect(h.workspace).toBe('parent')
expect(h.system).toContain('You review drafts in a Windmill workspace')
})
})
@@ -17,7 +17,8 @@ import {
reviewSystemPrompt,
VALUE_WITHHELD_KINDS,
type AiReviewFinding,
type FolderInstruction
type FolderInstruction,
type ReviewSubject
} from './aiReviewCore'
export {
@@ -41,32 +42,88 @@ export type AiReviewResult = {
instructionsUnavailable: boolean
}
/** One changed item to review, its two sides read only once the review gets to it. */
export type ReviewItem = {
kind: string
path: string
change: 'added' | 'deleted' | 'modified'
/** Resolves `undefined`, or throws, when the contents could not be read. */
load: () => Promise<{ before: unknown; after: unknown } | undefined>
}
/**
* What a review covers: the changes a fork carries into its parent, or drafts not yet deployed
* in their own workspace.
*/
export type ReviewScope =
| {
kind: 'deployment'
diffs: WorkspaceItemDiff[]
forkWorkspaceId: string
parentWorkspaceId: string
}
| { kind: 'drafts'; workspaceId: string; items: ReviewItem[] }
/** The workspace whose items change: fixes belong there, and the AI call is bound to it. */
export function scopeWorkspace(scope: ReviewScope): string {
return scope.kind === 'deployment' ? scope.forkWorkspaceId : scope.workspaceId
}
/**
* The workspace review instructions are read from. A fork's changes are reviewed under its
* parent's, never the fork's own: the fork is the author's side, and its copy of an instruction
* is itself one of the changes under review. Drafts are reviewed under their workspace's
* deployed instructions, for the same reason a draft of one does not apply.
*/
export function instructionsWorkspace(scope: ReviewScope): string {
return scope.kind === 'deployment' ? scope.parentWorkspaceId : scope.workspaceId
}
export function scopeItems(scope: ReviewScope): ReviewItem[] {
if (scope.kind === 'drafts') return scope.items
const { forkWorkspaceId, parentWorkspaceId } = scope
return scope.diffs.map((diff) => {
const change = changeType(diff)
return {
kind: diff.kind,
path: diff.path,
change,
load: async () => {
const kind = diff.kind as Kind
const [before, after] = 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.
return isEmptyRead(before) || isEmptyRead(after) ? undefined : { before, after }
}
}
})
}
const FETCH_BATCH = 6
// Read from the parent, never the fork: the fork is the author's side, and its copy of an
// instruction is itself one of the changes under review. A failed listing reviews without
// instructions rather than failing the review, and is reported as `unavailable`.
// A failed listing reviews without instructions rather than failing the review, and is reported
// as `unavailable`.
async function reviewInstructions(
diffs: WorkspaceItemDiff[],
parentWorkspaceId: string
subject: ReviewSubject,
paths: string[],
workspaceId: string
): Promise<{ text: string; applied: string[]; skipped: string[]; unavailable: boolean }> {
let listed: FolderInstruction[]
try {
listed = await listFolderInstructions(parentWorkspaceId, REVIEW_INSTRUCTIONS_RESOURCE_TYPE)
listed = await listFolderInstructions(workspaceId, REVIEW_INSTRUCTIONS_RESOURCE_TYPE)
} catch (e) {
console.error('Failed to list review instructions', e)
return { text: '', applied: [], skipped: [], unavailable: true }
}
const covering = instructionsCovering(
listed,
diffs.map((d) => d.path)
)
const covering = instructionsCovering(listed, paths)
const reads = await Promise.all(
covering.map(async (instruction) => {
try {
return {
instruction,
body: await readFolderInstructionBody(parentWorkspaceId, instruction.path)
body: await readFolderInstructionBody(workspaceId, instruction.path)
}
} catch (e) {
console.error(`Failed to read review instructions ${instruction.path}`, e)
@@ -78,7 +135,7 @@ async function reviewInstructions(
(r): r is { instruction: FolderInstruction; body: string } => r.body !== undefined
)
return {
text: reviewInstructionsSection('deployment_request', blocks),
text: reviewInstructionsSection(subject, blocks),
applied: blocks.map((b) => b.instruction.path),
skipped: reads.filter((r) => r.body === undefined).map((r) => r.instruction.path),
unavailable: false
@@ -91,27 +148,30 @@ function changeType(diff: WorkspaceItemDiff): 'added' | 'deleted' | 'modified' {
return 'modified'
}
const SIDES: Record<ReviewScope['kind'], [string, string]> = {
deployment: ['Parent version', 'Fork version'],
drafts: ['Deployed version', 'Draft version']
}
async function itemSection(
diff: WorkspaceItemDiff,
forkWorkspaceId: string,
parentWorkspaceId: string
item: ReviewItem,
sides: [string, string]
): Promise<{ text: string; unreadable: boolean }> {
const change = changeType(diff)
const header = `### ${diff.kind} ${diff.path} (${change})`
if (VALUE_WITHHELD_KINDS.has(diff.kind)) {
const header = `### ${item.kind} ${item.path} (${item.change})`
if (VALUE_WITHHELD_KINDS.has(item.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)) {
const values = await item.load().catch((e) => {
console.error(`Failed to read ${item.kind} ${item.path} for review`, e)
return undefined
})
if (!values) {
return { text: `${header}\nContents could not be read.`, unreadable: true }
}
const before = item.change === 'added' ? undefined : values.before
const after = item.change === 'deleted' ? undefined : values.after
return {
text: `${header}\n#### Parent version\n${renderItemValue(parent)}\n#### Fork version\n${renderItemValue(fork)}`,
text: `${header}\n#### ${sides[0]}\n${renderItemValue(before)}\n#### ${sides[1]}\n${renderItemValue(after)}`,
unreadable: false
}
}
@@ -120,30 +180,33 @@ function isEmptyRead(value: unknown): boolean {
return value != null && typeof value === 'object' && Object.keys(value as object).length === 0
}
/** 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,
/** Reviews the changes `scope` covers with the workspace's AI model. */
export async function reviewChanges(
scope: ReviewScope,
abortController: AbortController,
options?: {
/** Defaults to the chat's current model. */
model?: AIProviderModel
}
): Promise<AiReviewResult> {
const instructionsPromise = reviewInstructions(diffs, parentWorkspaceId)
const subject: ReviewSubject = scope.kind === 'deployment' ? 'deployment_request' : 'draft'
const workspaceId = scopeWorkspace(scope)
const items = scopeItems(scope)
const instructionsPromise = reviewInstructions(
subject,
items.map((i) => i.path),
instructionsWorkspace(scope)
)
const budget = new ReviewBudget()
const unreadable: string[] = []
// Fetched a batch at a time so a large fork stops reading items once the budget is spent.
for (let i = 0; i < diffs.length; i += FETCH_BATCH) {
const batch = diffs.slice(i, i + FETCH_BATCH)
// Fetched a batch at a time so a large review stops reading items once the budget is spent.
for (let i = 0; i < items.length; i += FETCH_BATCH) {
const batch = items.slice(i, i + FETCH_BATCH)
if (budget.spent) {
budget.omitted.push(...batch.map((d) => `${d.kind}:${d.path}`))
continue
}
const sections = await Promise.all(
batch.map((d) => itemSection(d, forkWorkspaceId, parentWorkspaceId))
)
const sections = await Promise.all(batch.map((d) => itemSection(d, SIDES[scope.kind])))
if (abortController.signal.aborted) throw new Error('aborted')
sections.forEach(({ text, unreadable: failed }, j) => {
const key = `${batch[j].kind}:${batch[j].path}`
@@ -156,26 +219,28 @@ export async function reviewDeployment(
if (abortController.signal.aborted) throw new Error('aborted')
const answer = await getNonStreamingCompletion(
[
{ role: 'system', content: reviewSystemPrompt('deployment_request') + instructions.text },
{ role: 'system', content: reviewSystemPrompt(subject) + instructions.text },
{
role: 'user',
content: budget.userPrompt(
`Fork workspace: ${forkWorkspaceId}\nParent workspace: ${parentWorkspaceId}`
scope.kind === 'deployment'
? `Fork workspace: ${scope.forkWorkspaceId}\nParent workspace: ${scope.parentWorkspaceId}`
: `Workspace: ${scope.workspaceId}`
)
}
],
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.
// Bound to the reviewed workspace: 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
workspace: workspaceId
}
)
return {
...parseReviewAnswer(answer ?? '', {
items: new Set(diffs.map((d) => `${d.kind}:${d.path}`))
items: new Set(items.map((d) => `${d.kind}:${d.path}`))
}),
omitted: budget.omitted,
unreadable,
@@ -1,5 +1,5 @@
// What the review asks of the model and how its answer is read, for every place it runs: the
// deployment request panel here, and `wmill review` on a git sync pull request, which carries
// deployment request panel and the drafts review here, and `wmill review` on a git sync pull request, which carries
// this module through cli/generate-ai-review.ts. Keep it free of `$lib` imports; the
// staleness test in cli/test/ai_review_bundle_unit.test.ts fails when the CLI's copy drifts.
import {
@@ -43,32 +43,47 @@ export const TOTAL_CHAR_BUDGET = 160_000
// Under the Anthropic SDK's non-streaming pre-flight max_tokens limit.
export const MAX_OUTPUT_TOKENS = 8_000
/** What is being reviewed: a fork's deployment request, or a pull request on a git sync repo. */
export type ReviewSubject = 'deployment_request' | 'pull_request'
/**
* What is being reviewed: a fork's deployment request, a workspace's undeployed drafts, or a
* pull request on a git sync repo.
*/
export type ReviewSubject = 'deployment_request' | 'draft' | 'pull_request'
const INTRO: Record<ReviewSubject, string> = {
deployment_request:
'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).',
draft:
'You review drafts in a Windmill workspace: changes saved to items of the workspace but not deployed yet. Each changed item is given with its deployed version (what is deployed now) and its draft version (what will be deployed).',
pull_request:
"You review a pull request on a git repository that Windmill's git sync keeps in step with a workspace: merging it deploys its changes. Each changed item is given with the files that make it up in the repository, each as a unified diff of the base branch (what is deployed now) against the pull request (what will be deployed). Every diff line starts with its line number: R<n> for line n of the pull request's version of the file, L<n> for line n of the base version, then + for an added line, - for a removed one and a space for unchanged context."
}
const ITEM_ANSWER_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>"}]}'
const ANSWER_SHAPE: Record<ReviewSubject, string> = {
deployment_request:
'{"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>"}]}',
deployment_request: ITEM_ANSWER_SHAPE,
draft: ITEM_ANSWER_SHAPE,
pull_request:
'{"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>", "file": "<the file the issue is in, as given>", "line": "<the R or L line number the issue is on, as given, such as R12, or null for the file as a whole>", "severity": "high" | "medium" | "low", "comment": "<the issue and what to do about it, under 80 words>"}]}'
}
const SOURCE: Record<ReviewSubject, string> = {
deployment_request: 'The parent workspace',
draft: 'The workspace',
pull_request: "The repository's base branch"
}
const APPROVED: Record<ReviewSubject, string> = {
deployment_request: 'the merge',
draft: 'the deployment',
pull_request: 'the merge'
}
export function reviewSystemPrompt(subject: ReviewSubject): string {
return `${INTRO[subject]} 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:
Report only issues a reviewer should act on before approving ${APPROVED[subject]}:
- 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
@@ -0,0 +1,21 @@
import { getDraftDiffValues } from '$lib/utils_draft_deploy'
import type { UserDraftItemKind } from '$lib/gen'
import type { ReviewItem } from './aiReview'
/** A draft to review against its deployed version, read the way its diff is shown. */
export function draftReviewItem(
kind: UserDraftItemKind,
path: string,
workspaceId: string,
draftOnly: boolean
): ReviewItem {
return {
kind,
path,
change: draftOnly ? 'added' : 'modified',
load: async () => {
const { deployed, draft } = await getDraftDiffValues(kind, path, workspaceId, draftOnly)
return { before: deployed, after: draft }
}
}
}
@@ -17,6 +17,15 @@ export type DiffVersionOption = {
* this is the picker's own appetite, not their default. */
export const VERSION_PAGE_SIZE = 20
/** Offers an AI review of the shown change, for a diff of a draft against its deployed version. */
export type DiffAiReview = {
workspaceId: string
kind: string
path: string
/** Never deployed: the left side is a placeholder, not a version. */
added?: boolean
}
export type DiffDrawerDiff =
| {
mode: 'normal'
@@ -52,6 +61,7 @@ export type DiffDrawerDiff =
current: Value
defaultDiffType?: 'deployed' | 'draft'
button?: { text: string; onClick: () => void }
aiReview?: DiffAiReview
}
| {
mode: 'simple'
@@ -59,6 +69,7 @@ export type DiffDrawerDiff =
current: Value
title: string
button?: { text: string; onClick: () => void }
aiReview?: DiffAiReview
}
export interface DiffDrawerI {
@@ -604,7 +604,10 @@
// reintroduce the post-deploy noise the head side already strips.
return replaceFalseWithUndefined(stripRawAppDiffNoise(v as any))
},
current: currentDiffValue
current: currentDiffValue,
aiReview: opWorkspace
? { workspaceId: opWorkspace, kind: 'raw_app', path: appPath }
: undefined
},
opening
)
@@ -870,6 +873,9 @@
mode: 'normal',
deployed: deployedValue ?? stripRawAppDiffNoise(savedApp),
current: currentDiffValue,
aiReview: opWorkspace
? { workspaceId: opWorkspace, kind: 'raw_app', path: appPath }
: undefined,
button: {
text: 'Looks good, deploy',
onClick: () => {
@@ -102,6 +102,7 @@
iconKindFor={(item) => (isAgent(item) ? 'agent' : undefined)}
{compareSessionHref}
workspaceLabel={ws?.name ?? workspaceId}
reviewWorkspaceId={workspaceId}
>
{#snippet titleExtra()}
<div class="flex items-center gap-1.5 text-xs text-secondary min-w-0">
@@ -42,6 +42,9 @@
import { maskKey } from './modifiedItemsMask'
import type { SessionDeployModel, DiffValues } from './sessionDeployModel.svelte'
import { SIDE_BY_SIDE_MIN_WIDTH } from '$lib/components/diffEditorTypes'
import { copilotInfo } from '$lib/aiStore'
import AiReview from '$lib/components/deploymentRequest/AiReview.svelte'
import type { ReviewItem } from '$lib/components/deploymentRequest/aiReview'
// The session "Edits" surface. Reads a unified item model (the session's
// drafts, scoped to what this session edited) and renders a folder tree
@@ -56,6 +59,7 @@
titleExtra,
compareSessionHref,
workspaceLabel,
reviewWorkspaceId,
successHoldMs = 1200
}: {
model: SessionDeployModel
@@ -73,8 +77,23 @@
/** Compare page for this session's edits (footer) — preselected via
* `from_session`. Where "deploy all / request review" happens. */
compareSessionHref?: string
/** The workspace the items live in; when set, the pending drafts can be reviewed with AI. */
reviewWorkspaceId?: string
} = $props()
// Your own pending drafts, read through the same loader as the diff column.
const reviewItems: ReviewItem[] = $derived(
model.items
.filter((i) => i.hasDraft && !i.done && i.mine)
.map((i) => ({
kind: i.draftKind,
path: i.path,
change: i.draftOnly ? 'added' : 'modified',
load: () => model.loadDiffValues(i)
}))
)
const showReview = $derived(!!reviewWorkspaceId && $copilotInfo.enabled && model.items.length > 0)
let drawer: Drawer | undefined = $state(undefined)
let searchQuery = $state('')
// The user's explicit view choice. The *effective* view can still be inline
@@ -1138,6 +1157,16 @@
</div>
</main>
</div>
{#if showReview && reviewWorkspaceId}
<div
class="shrink-0 border-t border-light bg-surface px-3 py-2 max-h-[40vh] overflow-y-auto"
>
<AiReview
scope={{ kind: 'drafts', workspaceId: reviewWorkspaceId, items: reviewItems }}
source="session_edits"
/>
</div>
{/if}
<!-- Batch / PR (deploy all, request review, reconcile) is the compare page's
job — the dock deploys per item; this footer is just the doorway there.
A plain anchor (not Button href): it's a text-level navigation link, and