From e20a27745a08d552e6d2c5a8bbaf08ccfe89c68f Mon Sep 17 00:00:00 2001 From: Guilhem Date: Mon, 22 Jun 2026 13:15:14 +0200 Subject: [PATCH] fix(frontend): strip raw-app post-deploy diff noise (raw_app/lock/data) (#9706) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(frontend): strip raw-app post-deploy diff noise (raw_app/lock/data) The raw-app editor's Diff drawer showed a spurious deployed-vs-current diff immediately after deploy, even with no edits: `raw_app: true`, a server-recomputed inline-script `lock`, and an empty `data` mismatch. These come from comparing the deployed app row (from getAppByPath) against the editor's current value, which differ on server-managed fields the editor never carries, on inline-script locks (recomputed at deploy, cleared on edit), and on `data` (the deployed row omits an empty `data` while the editor always carries the default `{tables: []}`). Add `stripRawAppDiffNoise` (strip server columns, null inline locks, canonicalize data) and apply it symmetrically to both diff sides in the editor header. For the session/compare draft diff, the draft is stored flat (files/runnables/data top-level) while the deployed row nests under `value`, so add `canonicalRawAppDiffValue` (= appSourceToDraftValue + stripRawAppDiffNoise) and route both sides through it in getDraftDiffValues. Both diff surfaces now share the same normalizer. Co-Authored-By: Claude Opus 4.8 (1M context) * fix(frontend): use canonicalized current value in deploy-drawer raw-app diff The Deploy drawer's "Diff" action still built the current side inline from raw editor state, bypassing stripRawAppDiffNoise — so inline-script `lock` and data-shape noise could resurface via Deploy → Diff even though the top-level Diff button was already fixed. Route it through `currentDiffValue` (and strip the savedApp fallback) so both entry points behave identically. Addresses Codex review finding on PR #9706. Co-Authored-By: Claude Opus 4.8 (1M context) --------- Co-authored-by: Claude Opus 4.8 (1M context) --- .../raw_apps/RawAppEditorHeader.svelte | 63 +++++------- .../src/lib/components/raw_apps/utils.test.ts | 99 +++++++++++++++++++ frontend/src/lib/components/raw_apps/utils.ts | 62 ++++++++++++ frontend/src/lib/utils_draft_deploy.ts | 11 +++ 4 files changed, 194 insertions(+), 41 deletions(-) diff --git a/frontend/src/lib/components/raw_apps/RawAppEditorHeader.svelte b/frontend/src/lib/components/raw_apps/RawAppEditorHeader.svelte index 2ecbc9f369..d1e52345bc 100644 --- a/frontend/src/lib/components/raw_apps/RawAppEditorHeader.svelte +++ b/frontend/src/lib/components/raw_apps/RawAppEditorHeader.svelte @@ -68,6 +68,7 @@ // `runtime.syncPreviewWithDeployed`, which discards the fork draft + reloads // the preview to the deployed version. import { AIBtnClasses } from '../copilot/chat/AIButtonStyle' + import { stripRawAppDiffNoise } from './utils' import type { RawAppData } from './dataTableRefUtils' import { isRuleActive } from '$lib/workspaceProtectionRules.svelte' import { buildForkEditUrl } from '$lib/utils/editInFork' @@ -369,15 +370,7 @@ savedApp && app && orderedJsonStringify(deployedValue) === - orderedJsonStringify( - replaceFalseWithUndefined({ - summary: summary, - value: app, - path: newEditedPath || savedApp.path, - policy, - custom_path: customPath - }) - ) + orderedJsonStringify(replaceFalseWithUndefined(currentDiffValue)) ) { await updateApp(npath) } else { @@ -400,15 +393,9 @@ deployedBy = deployedApp.created_by - // Strip off extra information - deployedValue = replaceFalseWithUndefined({ - ...deployedApp, - id: undefined, - created_at: undefined, - created_by: undefined, - versions: undefined, - extra_perms: undefined - }) + // Normalize away post-deploy noise (see stripRawAppDiffNoise) so the + // diff/comparison only reflects what the editor actually changed. + deployedValue = replaceFalseWithUndefined(stripRawAppDiffNoise(deployedApp)) } async function openDiffDrawer() { @@ -422,14 +409,8 @@ diffDrawer?.openDrawer() diffDrawer?.setDiff({ mode: 'normal', - deployed: deployedValue ?? savedApp, - current: { - summary: summary, - value: app, - path: newEditedPath || savedApp.path, - policy, - custom_path: customPath - } + deployed: deployedValue ?? stripRawAppDiffNoise(savedApp), + current: currentDiffValue }) } @@ -594,6 +575,18 @@ let app = $derived(files ? { runnables: runnables, files, data } : undefined) + // Editor-side value for diffing/comparison against the deployed app, with the + // same noise stripped as the deployed side (see stripRawAppDiffNoise). + let currentDiffValue = $derived( + stripRawAppDiffNoise({ + summary: summary, + value: app, + path: newEditedPath || savedApp?.path, + policy, + custom_path: customPath + }) + ) + $effect(() => { saveDrawerOpen && compareVersions() }) @@ -605,13 +598,7 @@ bind:open {diffDrawer} bind:deployedValue - currentValue={{ - summary: summary, - value: app, - path: newEditedPath || savedApp?.path, - policy, - custom_path: customPath - }} + currentValue={currentDiffValue} /> @@ -632,14 +619,8 @@ diffDrawer?.openDrawer() diffDrawer?.setDiff({ mode: 'normal', - deployed: deployedValue ?? savedApp, - current: { - summary: summary, - value: app, - path: newEditedPath || savedApp.path, - policy, - custom_path: customPath - }, + deployed: deployedValue ?? stripRawAppDiffNoise(savedApp), + current: currentDiffValue, button: { text: 'Looks good, deploy', onClick: () => { diff --git a/frontend/src/lib/components/raw_apps/utils.test.ts b/frontend/src/lib/components/raw_apps/utils.test.ts index b8ca6d207d..7d087a61ec 100644 --- a/frontend/src/lib/components/raw_apps/utils.test.ts +++ b/frontend/src/lib/components/raw_apps/utils.test.ts @@ -1,9 +1,11 @@ import { describe, expect, it } from 'vitest' import { + canonicalRawAppDiffValue, formatRuntimeLogsForChat, genWmillTs, normalizeRawAppRuntimeLogs, + stripRawAppDiffNoise, type Runnable } from './utils' @@ -61,3 +63,100 @@ describe('normalizeRawAppRuntimeLogs', () => { expect(formatRuntimeLogsForChat(entries)).toBe('[06:13:20.000] LOG: ready') }) }) + +// A deployed raw-app row as returned by getAppByPath: nested `value`, plus the +// server-managed columns and a recomputed inline-script lock. +function deployedRow() { + return { + id: 42, + raw_app: true, + is_draft: false, + created_at: '2024-01-01', + created_by: 'admin', + versions: [1, 2], + extra_perms: { 'u/admin': true }, + summary: 'app', + path: 'u/admin/app', + policy: { execution_mode: 'publisher' }, + value: { + files: { '/App.tsx': 'export default 1' }, + runnables: { + a: { type: 'inline', inlineScript: { content: 'main()', language: 'bun', lock: 'deps\n' } } + } + } + } +} + +describe('stripRawAppDiffNoise', () => { + it('drops server-managed columns, nulls inline locks and canonicalizes data', () => { + const cleaned = stripRawAppDiffNoise(deployedRow()) + + for (const key of [ + 'raw_app', + 'id', + 'created_at', + 'created_by', + 'versions', + 'extra_perms', + 'is_draft' + ]) { + expect(cleaned).not.toHaveProperty(key) + } + expect(cleaned.value.runnables.a.inlineScript.lock).toBeUndefined() + // absent `data` is canonicalized to the default empty shape + expect(cleaned.value.data).toEqual({ tables: [], datatable: undefined, schema: undefined }) + }) + + it('does not mutate the input (live editor state)', () => { + const input = deployedRow() + stripRawAppDiffNoise(input) + expect(input.raw_app).toBe(true) + expect(input.value.runnables.a.inlineScript.lock).toBe('deps\n') + }) + + it('handles the flat editor/draft shape (files/runnables top-level)', () => { + const flat = { + summary: 'app', + files: { '/App.tsx': 'x' }, + runnables: { + a: { type: 'inline', inlineScript: { content: 'm()', language: 'bun', lock: 'l' } } + } + } + const cleaned = stripRawAppDiffNoise(flat) + expect(cleaned.runnables.a.inlineScript.lock).toBeUndefined() + expect(cleaned.data).toEqual({ tables: [], datatable: undefined, schema: undefined }) + }) +}) + +describe('canonicalRawAppDiffValue', () => { + it('collapses a nested deployed row and a flat draft to an identical value when content matches', () => { + const deployed = deployedRow() + // The flat draft shape a raw app autosaves: top-level files/runnables/data, + // no server columns, lock cleared on edit. + const draft = { + summary: 'app', + files: { '/App.tsx': 'export default 1' }, + runnables: { a: { type: 'inline', inlineScript: { content: 'main()', language: 'bun' } } }, + data: { tables: [] }, + policy: { execution_mode: 'publisher' } + } + + expect(canonicalRawAppDiffValue(deployed)).toEqual(canonicalRawAppDiffValue(draft)) + }) + + it('still surfaces a real change (summary edit)', () => { + const deployed = deployedRow() + const draft = { + summary: 'app EDITED', + files: { '/App.tsx': 'export default 1' }, + runnables: { a: { type: 'inline', inlineScript: { content: 'main()', language: 'bun' } } }, + data: { tables: [] } + } + + const a = canonicalRawAppDiffValue(deployed) + const b = canonicalRawAppDiffValue(draft) + expect(a).not.toEqual(b) + expect(a.summary).toBe('app') + expect(b.summary).toBe('app EDITED') + }) +}) diff --git a/frontend/src/lib/components/raw_apps/utils.ts b/frontend/src/lib/components/raw_apps/utils.ts index 53d84fd97c..9558697708 100644 --- a/frontend/src/lib/components/raw_apps/utils.ts +++ b/frontend/src/lib/components/raw_apps/utils.ts @@ -3,6 +3,8 @@ import type { Schema } from '../../common' import { schemaToTsType } from '../../schema' import { isRunnableByName, isRunnableByPath, type RunnableWithFields } from '../apps/inputType' import type { InlineScript } from '../apps/sharedTypes' +import { stateSnapshot } from '$lib/svelte5Utils.svelte' +import { appSourceToDraftValue, normalizeRawAppData } from './rawAppDraftValue' // export type RunnableWithFields = any @@ -15,6 +17,66 @@ export type RawApp = { files: string[] } +// Server-managed columns the deployed app row (getAppByPath) carries but the +// editor's current value never does — leaving them in renders as spurious diff. +const RAW_APP_DEPLOYED_METADATA_KEYS = [ + 'raw_app', + 'id', + 'created_at', + 'created_by', + 'versions', + 'extra_perms', + 'is_draft' +] as const + +/** + * Normalize a raw-app value before a deployed-vs-current diff (or unsaved-change + * comparison). Three sources of spurious post-deploy diff: + * - the deployed row carries server-managed columns (`raw_app`, timestamps, …) + * that the editor's current value lacks; + * - inline-script `lock`s are recomputed server-side at every deploy and the + * editor clears them on edit, so the editor value and the freshly deployed + * one always diverge on `lock` even though the user changed nothing there; + * - the deployed value omits an empty `data` while the editor always carries + * the default `{ tables: [] }`, so even an untouched app reads as changed. + * All three must be neutralized symmetrically on both sides. Returns a deep + * clone; never mutates the input (the current side is live editor state). + */ +export function stripRawAppDiffNoise>(value: T): T { + const cloned = structuredClone(stateSnapshot(value)) as Record + for (const key of RAW_APP_DEPLOYED_METADATA_KEYS) { + delete cloned[key] + } + // Runnables/data live under `.value` on a deployed row and on the editor's + // diff value alike, but a flat draft shape carries them top-level. + const source = cloned.value ?? cloned + const runnables = source.runnables + if (runnables && typeof runnables === 'object') { + for (const k of Object.keys(runnables)) { + const inlineScript = runnables[k]?.inlineScript + if (inlineScript && inlineScript.lock != undefined) { + inlineScript.lock = undefined + } + } + } + // Canonicalize `data` so an absent and a default-empty `data` compare equal. + source.data = normalizeRawAppData(source) + return cloned as T +} + +/** + * Canonical raw-app value for diffing a *draft* against a *deployed* row. On top + * of the noise stripped by stripRawAppDiffNoise, the two also differ in shape: a + * deployed row nests its source under `value`, whereas a draft carries + * `files`/`runnables`/`data` at the top level. `appSourceToDraftValue` collapses + * both onto the same flat field set first. Use this for the session/compare + * draft diff so it matches the editor's Diff button (which shares + * stripRawAppDiffNoise). + */ +export function canonicalRawAppDiffValue(source: Record) { + return stripRawAppDiffNoise(appSourceToDraftValue(source)) +} + export type RawAppRuntimeLogLevel = 'log' | 'info' | 'warn' | 'error' | 'debug' export type RawAppRuntimeLogEntry = { level: RawAppRuntimeLogLevel diff --git a/frontend/src/lib/utils_draft_deploy.ts b/frontend/src/lib/utils_draft_deploy.ts index 47d9e1096b..751f42e3cb 100644 --- a/frontend/src/lib/utils_draft_deploy.ts +++ b/frontend/src/lib/utils_draft_deploy.ts @@ -37,6 +37,7 @@ import { UserDraftDbSyncer } from '$lib/userDraftDbSyncer.svelte' import type { DeployResult } from '$lib/utils_workspace_deploy' import { TRIGGER_RUNTIME_IGNORE } from '$lib/utils_deployable' import { deployRawAppDraft } from '$lib/rawAppDeploy' +import { canonicalRawAppDiffValue } from '$lib/components/raw_apps/utils' import { invalidateWorkspaceDrafts } from '$lib/workspaceDrafts.svelte' import { setLocalDraftHint } from '$lib/localDraftHints.svelte' import { userStore } from '$lib/stores' @@ -219,6 +220,16 @@ export async function getDraftDiffValues( getDraft: true, rawApp: kind === 'raw_app' })) as any + if (kind === 'raw_app' || r.raw_app === true) { + // Raw-app drafts are stored flat (files/runnables/data top-level) while the + // deployed row nests them under `value`, and deployed inline scripts carry + // server-recomputed locks. Canonicalize both onto the same shape with the + // post-deploy noise stripped — the same module the editor's Diff button uses. + return { + deployed: draftOnly ? canonicalRawAppDiffValue({}) : canonicalRawAppDiffValue(r), + draft: canonicalRawAppDiffValue(r.draft ?? r) + } + } const deployed = { summary: r.summary, value: r.value,