diff --git a/.changeset/asset-create-only.md b/.changeset/asset-create-only.md new file mode 100644 index 00000000..39913181 --- /dev/null +++ b/.changeset/asset-create-only.md @@ -0,0 +1,5 @@ +--- +"@luzzle/core": patch +--- + +Prevent concurrent asset uploads from overwriting each other. If another upload creates the chosen filename first, fail safely instead of replacing that asset. diff --git a/.changeset/asset-url-schemes.md b/.changeset/asset-url-schemes.md new file mode 100644 index 00000000..ff3ee2a6 --- /dev/null +++ b/.changeset/asset-url-schemes.md @@ -0,0 +1,5 @@ +--- +"@luzzle/web": patch +--- + +Recognize HTTP(S) asset URLs regardless of scheme capitalization when saving piece source, converting them to local assets before validation. diff --git a/.changeset/editor-generation.md b/.changeset/editor-generation.md new file mode 100644 index 00000000..2d4d47f7 --- /dev/null +++ b/.changeset/editor-generation.md @@ -0,0 +1,12 @@ +--- +"@luzzle/core": patch +"@luzzle/cli": patch +"@luzzle/web": patch +"@luzzle/web.proxy": patch +--- + +Generate one or more metadata fields, or append to the Markdown body, directly from the source editor. Metadata generation starts with all fields selected and the field picker focused. Generation shows inline progress and applies one undoable, unsaved document update. Creation stays a simple create-and-edit flow; the separate generator page is retired. + +The core generation APIs are `generatePieceFrontmatter` and `generatePieceBody`, with `FrontmatterGenerationRequest` describing a metadata request. The CLI assistant calls `generatePieceFrontmatter` directly; the obsolete `pieceFrontMatterFromPrompt` export is removed. Command options and final-only output are unchanged. + +Allow HTTP(S) asset URLs in generated content and resolve them on Save. Preserve strict stored-piece validation. Share SSE transport without sharing generation and workflow progress screens. diff --git a/apps/cli/src/lib/commands/command/assistant.test.ts b/apps/cli/src/lib/commands/command/assistant.test.ts index afa824f1..a874087e 100644 --- a/apps/cli/src/lib/commands/command/assistant.test.ts +++ b/apps/cli/src/lib/commands/command/assistant.test.ts @@ -8,7 +8,7 @@ import yargs from 'yargs' import { makeContext, makeMarkdownSample, makePieceMock } from '../utils/context.fixtures.js' import { makePieceOption, parsePieceOptionArgv } from '../utils/pieces.js' import yaml from 'yaml' -import { pieceFrontMatterFromPrompt } from '@luzzle/core' +import { generatePieceFrontmatter } from '@luzzle/core' vi.mock('@luzzle/core') vi.mock('../utils/pieces.js') @@ -20,7 +20,7 @@ const mocks = { logInfo: vi.spyOn(log, 'info'), parseArgs: vi.mocked(parsePieceOptionArgv), makeOption: vi.mocked(makePieceOption), - generatePieceFrontmatter: vi.mocked(pieceFrontMatterFromPrompt), + generatePieceFrontmatter: vi.mocked(generatePieceFrontmatter), getPiece: vi.fn(), consoleLog: vi.spyOn(console, 'log'), yamlStringify: vi.mocked(yaml.stringify), @@ -40,7 +40,7 @@ describe('lib/commands/assistant.ts', () => { }) }) - test('run', async () => { + test.each([{ file: undefined }, { file: ['book.pdf', 'notes.txt'] }])('generates full metadata directly with files $file', async ({ file }) => { const apiKeys = 'api_key' const piece = makePieceMock() const frontmatter = makeMarkdownSample().frontmatter @@ -56,15 +56,14 @@ describe('lib/commands/assistant.ts', () => { frontmatter as unknown as Record ) - await command.run(ctx, { prompt } as Arguments) + await command.run(ctx, { prompt, file } as Arguments) expect(mocks.yamlStringify).toHaveBeenCalledOnce() expect(mocks.consoleLog).toHaveBeenCalledOnce() expect(mocks.generatePieceFrontmatter).toHaveBeenCalledWith( apiKeys, - piece.schema, - prompt, - undefined + { schema: piece.schema, instructions: prompt }, + { files: file } ) }) @@ -211,6 +210,40 @@ describe('lib/commands/assistant.ts', () => { expect(spies.pieceSetFields).toHaveBeenCalledWith(markdown, frontmatter) }) + test('stays quiet until the validated result is ready', async () => { + const piece = makePieceMock() + const pending = Promise.withResolvers>>() + mocks.parseArgs.mockResolvedValueOnce({ piece }) + mocks.generatePieceFrontmatter.mockReturnValueOnce(pending.promise) + const running = command.run(makeContext(), { prompt: 'Find details' } as Arguments) + await vi.waitFor(() => expect(mocks.generatePieceFrontmatter).toHaveBeenCalledOnce()) + expect(mocks.consoleLog).not.toHaveBeenCalled() + expect(mocks.logInfo).not.toHaveBeenCalled() + expect(mocks.yamlStringify).not.toHaveBeenCalled() + pending.resolve({ title: 'Completed' }) + await running + expect(mocks.yamlStringify).toHaveBeenCalledWith({ title: 'Completed' }) + expect(mocks.consoleLog).toHaveBeenCalledOnce() + }) + + test.each([ + {}, + { update: 'existing.books.md' }, + { directory: 'books', title: 'New book' }, + ])('does not apply or print a failed generation: %j', async (options) => { + const piece = makePieceMock() + mocks.parseArgs.mockResolvedValueOnce({ piece }) + mocks.generatePieceFrontmatter.mockRejectedValueOnce(new Error('Invalid generated metadata')) + await expect(command.run(makeContext(), { prompt: 'Find details', ...options } as Arguments)) + .rejects.toThrow('Invalid generated metadata') + expect(piece.get).not.toHaveBeenCalled() + expect(piece.create).not.toHaveBeenCalled() + expect(piece.setFields).not.toHaveBeenCalled() + expect(piece.write).not.toHaveBeenCalled() + expect(mocks.consoleLog).not.toHaveBeenCalled() + expect(mocks.yamlStringify).not.toHaveBeenCalled() + }) + test('builder', async () => { const args = yargs() diff --git a/apps/cli/src/lib/commands/command/assistant.ts b/apps/cli/src/lib/commands/command/assistant.ts index 7092994e..25373097 100644 --- a/apps/cli/src/lib/commands/command/assistant.ts +++ b/apps/cli/src/lib/commands/command/assistant.ts @@ -3,7 +3,7 @@ import type { Command } from '../utils/types.js' import yaml from 'yaml' import type { PieceArgv} from '../utils/pieces.js'; import { makePieceOption, parsePieceOptionArgv } from '../utils/pieces.js' -import { pieceFrontMatterFromPrompt } from '@luzzle/core' +import { generatePieceFrontmatter } from '@luzzle/core' export type AssistantArgv = { update?: string @@ -67,7 +67,11 @@ const command: Command = { const { piece } = await parsePieceOptionArgv(ctx, args) const apiKey = ctx.config.get('api_keys.google', '') - const metadata = await pieceFrontMatterFromPrompt(apiKey, piece.schema, prompt, file) + const metadata = await generatePieceFrontmatter( + apiKey, + { schema: piece.schema, instructions: prompt }, + { files: file } + ) if (update) { const markdown = await piece.get(update) diff --git a/apps/core/src/index.test.ts b/apps/core/src/index.test.ts index 417073a6..68e15d11 100644 --- a/apps/core/src/index.test.ts +++ b/apps/core/src/index.test.ts @@ -10,15 +10,21 @@ describe('index.ts', () => { test('preserves the public generation exports without exposing provider internals', () => { for (const name of [ - 'generatePieceMetadata', - 'generateFieldValue', - 'generateBody', - 'pieceFrontMatterFromPrompt', + 'generatePieceFrontmatter', + 'mergeGeneratedFields', + 'appendGeneratedBody', + 'generatePieceBody', 'validateApiKey', ] as const) { expect(index[name]).toBeTypeOf('function') } expect(index.DEFAULT_GENERATION_LIMITS).toBeDefined() + expect(Object.hasOwn(index, 'generatePieceMetadata')).toBe(false) + expect(Object.hasOwn(index, 'generateFieldValues')).toBe(false) + expect(Object.hasOwn(index, 'generateBody')).toBe(false) + expect(Object.hasOwn(index, 'pieceFrontMatterFromPrompt')).toBe(false) + expect(Object.hasOwn(index, 'generateFieldValue')).toBe(false) + expect(Object.hasOwn(index, 'replaceGeneratedBody')).toBe(false) expect(Object.hasOwn(index, 'getClient')).toBe(false) expect(Object.hasOwn(index, 'runGeneration')).toBe(false) }) diff --git a/apps/core/src/index.ts b/apps/core/src/index.ts index 8975fb19..35a50dc5 100644 --- a/apps/core/src/index.ts +++ b/apps/core/src/index.ts @@ -19,6 +19,7 @@ export * from './database/tables/index.js' export * from './pieces/index.js' export * from './llm/google/index.js' export * from './storage/index.js' +export { mergeGeneratedFields, appendGeneratedBody } from './pieces/utils/generation.js' export { getDatabaseClient, diff --git a/apps/core/src/lib/ajv.test.ts b/apps/core/src/lib/ajv.test.ts index 3f20e31a..03597ace 100644 --- a/apps/core/src/lib/ajv.test.ts +++ b/apps/core/src/lib/ajv.test.ts @@ -4,6 +4,7 @@ import { makeSchema } from '../pieces/Piece.fixtures.js' import Ajv from 'ajv' import ajv, { assetFormatValidator, + assetSourceFormatValidator, dateFormatValidator, commaSeparatedFormatValidator, paragraphFormatValidator, @@ -41,6 +42,52 @@ describe('src/lib/ajv.ts', () => { expect(mocks.compile).toHaveBeenCalledWith(schema) }) + test.each([undefined, {}, { allowAssetUrls: false }])( + 'keeps the strict asset format by default: %j', + (options) => { + ajv(makeSchema(), options) + expect(mocks.addFormat).toHaveBeenCalledWith('asset', { + type: 'string', validate: assetFormatValidator, + }) + } + ) + + test('draft compilation changes only the asset validator, not the schema or other formats', () => { + const schema = makeSchema({ cover: { type: 'string', format: 'asset' } }) + const original = structuredClone(schema) + ajv(schema, { allowAssetUrls: true }) + + expect(mocks.addFormat.mock.calls).toEqual([ + ['date', { type: 'string', validate: dateFormatValidator }], + ['asset', { type: 'string', validate: assetSourceFormatValidator }], + ['comma-separated', { type: 'string', validate: commaSeparatedFormatValidator }], + ['paragraph', { type: 'string', validate: paragraphFormatValidator }], + ['markdown', { type: 'string', validate: markdownFormatValidator }], + ]) + expect(mocks.compile).toHaveBeenCalledWith(schema) + expect(schema).toEqual(original) + }) + + test.each([ + '.assets/books/cover.jpg', + 'https://example.com/cover.jpg?size=large#cover', + 'http://example.com/cover.jpg', + 'HTTPS://example.com/cover.jpg', + 'HtTp://example.com/cover.jpg', + ])('assetSourceFormatValidator accepts %s', (source) => { + expect(assetSourceFormatValidator(source)).toBe(true) + if (!source.startsWith('.assets/')) expect(assetFormatValidator(source)).toBe(false) + }) + + test.each([ + '', 'not an asset', './cover.jpg', '.assets/cover.jpg', + 'https://', 'http://[invalid', '//example.com/cover.jpg', + 'file:///tmp/cover.jpg', 'data:image/png;base64,YQ==', 'javascript:alert(1)', + 'ftp://example.com/cover.jpg', ' https://example.com/cover.jpg', + ])('assetSourceFormatValidator rejects %s', (source) => { + expect(assetSourceFormatValidator(source)).toBe(false) + }) + test('commaSeparatedFormatValidator', () => { expect(commaSeparatedFormatValidator('a,b,c')).toBe(true) }) diff --git a/apps/core/src/lib/ajv.ts b/apps/core/src/lib/ajv.ts index 2edf96ce..f4ea298b 100644 --- a/apps/core/src/lib/ajv.ts +++ b/apps/core/src/lib/ajv.ts @@ -17,15 +17,25 @@ export function assetFormatValidator(data: string) { return /^\.assets\/.+\/.+/.test(data) } +export function assetSourceFormatValidator(data: string) { + return assetFormatValidator(data) || (/^https?:\/\//i.test(data) && URL.canParse(data)) +} + export function paragraphFormatValidator(data: string) { return typeof data === 'string' } -export default function (schema: JSONSchemaType): ValidateFunction { +export default function ( + schema: JSONSchemaType, + options?: { allowAssetUrls?: boolean } +): ValidateFunction { const ajv = new Ajv() ajv.addFormat('date', { type: 'string', validate: dateFormatValidator }) - ajv.addFormat('asset', { type: 'string', validate: assetFormatValidator }) + ajv.addFormat('asset', { + type: 'string', + validate: options?.allowAssetUrls ? assetSourceFormatValidator : assetFormatValidator, + }) ajv.addFormat('comma-separated', { type: 'string', validate: commaSeparatedFormatValidator, diff --git a/apps/core/src/llm/field-schema.test.ts b/apps/core/src/llm/field-schema.test.ts index 6b754225..a85f9929 100644 --- a/apps/core/src/llm/field-schema.test.ts +++ b/apps/core/src/llm/field-schema.test.ts @@ -2,7 +2,7 @@ import type { JSONSchemaType, SchemaObject } from 'ajv' import { describe, expect, test } from 'vitest' import compile from '../lib/ajv.js' import type { PieceFrontmatter } from '../pieces/utils/frontmatter.js' -import { selectFieldSchema } from './field-schema.js' +import { selectFieldsSchema } from './field-schema.js' const makeSchema = (field: unknown, extra: SchemaObject = {}) => ({ @@ -12,7 +12,7 @@ const makeSchema = (field: unknown, extra: SchemaObject = {}) => ...extra, }) as JSONSchemaType -describe('selectFieldSchema', () => { +describe('selectFieldsSchema', () => { test('requires only the selected property and rejects other output keys', () => { const source = makeSchema( { type: 'string', minLength: 2 }, @@ -22,7 +22,7 @@ describe('selectFieldSchema', () => { $defs: { unused: { type: 'integer' } }, } ) - const result = selectFieldSchema(source, 'field') + const result = selectFieldsSchema(source, ['field']) expect(result).toEqual({ type: 'object', properties: { field: { type: 'string', minLength: 2 } }, @@ -36,6 +36,51 @@ describe('selectFieldSchema', () => { } }) + test('selects many or all fields with exact required keys, including metadata named body', () => { + const source = makeSchema({ type: 'string', nullable: true }, { + properties: { + field: { type: 'string', nullable: true }, + body: { type: 'string', minLength: 2 }, + other: { type: 'integer', minimum: 1 }, + }, + }) + const original = structuredClone(source) + for (const keys of [['field', 'body'], Object.keys(source.properties)]) { + const selected = selectFieldsSchema(source, keys) + expect(Object.keys(selected.properties)).toEqual(keys) + expect(selected.required).toEqual(keys) + expect(selected.additionalProperties).toBe(false) + for (const key of keys) { + expect(selected.properties[key]).toEqual(source.properties[key]) + } + const values = { field: null, body: 'Metadata', ...(keys.includes('other') ? { other: 1 } : {}) } + const validate = compile(selected) + expect(validate(values)).toBe(true) + expect(validate({ ...values, extra: true })).toBe(false) + expect(validate({ ...values, body: '' })).toBe(false) + expect(validate({ ...values, field: 42 })).toBe(false) + for (const key of keys) { + const missing = { ...values } as Record + delete missing[key] + expect(validate(missing)).toBe(false) + } + } + expect(source).toEqual(original) + }) + + test('requires a nonempty selection and validates every key', () => { + const source = makeSchema({ type: 'string' }) + expect(() => selectFieldsSchema(source, [])).toThrow('nonempty') + expect(() => selectFieldsSchema(source, ['field', 'missing'])).toThrow('own top-level') + expect(() => selectFieldsSchema(source, [42 as unknown as string])).toThrow('own top-level') + }) + + test('treats repeated selections as a single required property', () => { + const selected = selectFieldsSchema(makeSchema({ type: 'string' }), ['field', 'field']) + expect(selected.required).toEqual(['field']) + expect(compile(selected)({ field: 'value' })).toBe(true) + }) + test('preserves inline array/object constraints, nullability, formats, and source data', () => { const source = makeSchema({ type: 'array', @@ -46,13 +91,12 @@ describe('selectFieldSchema', () => { }, }) const original = structuredClone(source) - const result = selectFieldSchema(source, 'field') + const result = selectFieldsSchema(source, ['field']) const validate = compile(result) expect(validate({ field: [{ date: null }, { date: '2026-01-02' }] })).toBe(true) expect(validate({ field: [{ date: 'invalid' }] })).toBe(false) expect(source).toEqual(original) expect(result.properties.field).toEqual(source.properties.field) - expect(result.properties.field).not.toBe(source.properties.field) }) test('does not mistake literal example data or property names for schema references', () => { @@ -65,32 +109,31 @@ describe('selectFieldSchema', () => { enum: [data], properties: { $ref: { type: 'string' }, $dynamicRef: { type: 'string' } }, }) - const result = selectFieldSchema(source, 'field') + const result = selectFieldsSchema(source, ['field']) expect(result.properties.field).toEqual(source.properties.field) - ;(result.properties.field as SchemaObject).default.$ref = 'changed' expect(data.$ref).toBe('#') }) test('selects literal own keys, not inherited or missing keys', () => { const source = makeSchema(true, { properties: { 'a/b~c': { type: 'string' } } }) - expect(selectFieldSchema(source, 'a/b~c').required).toEqual(['a/b~c']) + expect(selectFieldsSchema(source, ['a/b~c']).required).toEqual(['a/b~c']) for (const key of ['missing', 'toString', '__proto__']) { - expect(() => selectFieldSchema(source, key)).toThrow('own top-level property') + expect(() => selectFieldsSchema(source, [key])).toThrow('own top-level property') } expect(() => - selectFieldSchema({ type: 'object' } as JSONSchemaType, 'field') + selectFieldsSchema({ type: 'object' } as JSONSchemaType, ['field']) ).toThrow('own top-level property') }) test('rejects a reserved field name that the validator skips', () => { const source = makeSchema(true, { properties: JSON.parse('{"__proto__":{"type":"string"}}') }) - expect(() => selectFieldSchema(source, '__proto__')).toThrow('validator does not support') + expect(() => selectFieldsSchema(source, ['__proto__'])).toThrow('validator does not support') }) test.each(['$ref', '$dynamicRef', '$recursiveRef'])( 'rejects an unexpected %s instead of resolving it', (keyword) => { - expect(() => selectFieldSchema(makeSchema({ [keyword]: '#' }), 'field')).toThrow( + expect(() => selectFieldsSchema(makeSchema({ [keyword]: '#' }), ['field'])).toThrow( 'reference-free schemas' ) } @@ -107,12 +150,12 @@ describe('selectFieldSchema', () => { makeSchema({ type: 'string' }, { $defs: { unused: { $ref: '#' } } }), makeSchema({ type: 'string' }, { $ref: 'external.json' }), ])('rejects references anywhere in the input schema', (source) => { - expect(() => selectFieldSchema(source, 'field')).toThrow('reference-free schemas') + expect(() => selectFieldsSchema(source, ['field'])).toThrow('reference-free schemas') }) test('preserves the dialect for the normal Ajv compiler', () => { const dialect = 'http://json-schema.org/draft-07/schema#' - const result = selectFieldSchema(makeSchema({ type: 'string' }, { $schema: dialect }), 'field') + const result = selectFieldsSchema(makeSchema({ type: 'string' }, { $schema: dialect }), ['field']) expect(result.$schema).toBe(dialect) expect(compile(result)({ field: 'value' })).toBe(true) }) diff --git a/apps/core/src/llm/field-schema.ts b/apps/core/src/llm/field-schema.ts index 60d170e2..077d579b 100644 --- a/apps/core/src/llm/field-schema.ts +++ b/apps/core/src/llm/field-schema.ts @@ -2,15 +2,20 @@ import type { SchemaObject } from 'ajv' import traverse from 'json-schema-traverse' import type { PieceFrontmatter, PieceFrontmatterSchema } from '../pieces/utils/frontmatter.js' -export function selectFieldSchema( +export function selectFieldsSchema( schema: PieceFrontmatterSchema, - key: string + keys: string[] ): PieceFrontmatterSchema { - if (!schema.properties || !Object.hasOwn(schema.properties, key)) { - throw new Error('Field generation requires an own top-level property.') + if (!Array.isArray(keys) || keys.length === 0) { + throw new Error('Field generation requires a nonempty selection of fields.') + } + for (const key of keys) { + if (typeof key !== 'string' || !schema.properties || !Object.hasOwn(schema.properties, key)) { + throw new Error('Field generation requires an own top-level property.') + } + if (key === '__proto__') + throw new Error('The validator does not support a field named __proto__.') } - if (key === '__proto__') - throw new Error('The validator does not support a field named __proto__.') traverse(schema, { cb(node) { @@ -24,10 +29,10 @@ export function selectFieldSchema( const responseSchema: SchemaObject = { type: 'object', - properties: { [key]: schema.properties[key] }, - required: [key], + properties: Object.fromEntries(keys.map((key) => [key, schema.properties[key]])), + required: [...new Set(keys)], additionalProperties: false, } if (Object.hasOwn(schema, '$schema')) responseSchema.$schema = schema.$schema - return structuredClone(responseSchema) as PieceFrontmatterSchema + return responseSchema as PieceFrontmatterSchema } diff --git a/apps/core/src/llm/google/client.ts b/apps/core/src/llm/google/client.ts index 6d15e822..d4cd5583 100644 --- a/apps/core/src/llm/google/client.ts +++ b/apps/core/src/llm/google/client.ts @@ -28,11 +28,10 @@ export type GenerationOptions = { onProgress?: (progress: GenerationProgress) => void | Promise } -type GenerationTask = { +type GenerationTask = { prompt: string[] systemInstruction: string schema?: PieceFrontmatterSchema - decode: (text: string) => T } type GenerationContext = { @@ -93,7 +92,7 @@ async function uploadFile( }) } catch (error) { const status = error instanceof ApiError ? ` (HTTP ${error.status})` : '' - throw new Error(`Gemini file upload failed${status}.`) + throw new Error(`Gemini file upload failed${status}.`, { cause: error }) } if (typeof uploaded?.name !== 'string' || !uploaded.name) { throw new Error('Gemini file upload returned no file name.') @@ -192,42 +191,12 @@ function completedText(response: GenerateContentResponse): string { return text } -async function generateText( - context: GenerationContext, - task: GenerationTask, - attachments: Array -) { - progress(context, 'generation', 'Generating content.') - const response = await context.client.models.generateContent({ - model: MODEL_NAME, - contents: [...task.prompt, ...attachments], - config: { - candidateCount: 1, - systemInstruction: task.systemInstruction, - ...(task.schema - ? { responseMimeType: 'application/json', responseJsonSchema: task.schema } - : {}), - safetySettings: [ - { - category: HarmCategory.HARM_CATEGORY_SEXUALLY_EXPLICIT, - threshold: HarmBlockThreshold.BLOCK_NONE, - }, - { - category: HarmCategory.HARM_CATEGORY_DANGEROUS_CONTENT, - threshold: HarmBlockThreshold.BLOCK_NONE, - }, - ], - }, - }) - return completedText(response) -} - -export async function runGeneration( +export async function runGeneration( apiKey: string, - task: GenerationTask, + task: GenerationTask, options: GenerationOptions -): Promise { - const files = [...(options.files ?? [])] +): Promise { + const files = options.files ?? [] if (files.length > DEFAULT_GENERATION_LIMITS.maxFiles) throw new Error('Too many generation attachments.') const context: GenerationContext = { @@ -238,9 +207,30 @@ export async function runGeneration( try { progress(context, 'preparation', 'Preparing generation inputs.') const attachments = await prepareAttachments(context, files) - const text = await generateText(context, task, attachments) + progress(context, 'generation', 'Generating content.') + const response = await context.client.models.generateContent({ + model: MODEL_NAME, + contents: [...task.prompt, ...attachments], + config: { + candidateCount: 1, + systemInstruction: task.systemInstruction, + ...(task.schema + ? { responseMimeType: 'application/json', responseJsonSchema: task.schema } + : {}), + safetySettings: [ + { + category: HarmCategory.HARM_CATEGORY_SEXUALLY_EXPLICIT, + threshold: HarmBlockThreshold.BLOCK_NONE, + }, + { + category: HarmCategory.HARM_CATEGORY_DANGEROUS_CONTENT, + threshold: HarmBlockThreshold.BLOCK_NONE, + }, + ], + }, + }) progress(context, 'validation', 'Validating the completed result.') - return task.decode(text) + return completedText(response) } finally { await cleanupFiles(context) } diff --git a/apps/core/src/llm/google/generation.test.ts b/apps/core/src/llm/google/generation.test.ts index 98556938..1252b722 100644 --- a/apps/core/src/llm/google/generation.test.ts +++ b/apps/core/src/llm/google/generation.test.ts @@ -14,10 +14,9 @@ import { import type { PieceFrontmatter } from '../../pieces/utils/frontmatter.js' import { DEFAULT_GENERATION_LIMITS, - pieceFrontMatterFromPrompt, - generatePieceMetadata, - generateFieldValue, - generateBody, + GenerationValidationError, + generatePieceFrontmatter, + generatePieceBody, validateApiKey, } from './index.js' import type { GenerationOptions } from './index.js' @@ -83,7 +82,7 @@ async function until(condition: () => boolean) { // Small byte budgets exercise boundaries without allocating deployment-sized fixtures. const originalLimits = { ...DEFAULT_GENERATION_LIMITS } const run = (options: GenerationOptions = {}) => - generatePieceMetadata('test-key', schema(), 'My instructions', options) + generatePieceFrontmatter('test-key', { schema: schema(), instructions: 'My instructions' }, options) beforeEach(() => { Object.values(mocks).forEach((mock) => mock.mockReset()) @@ -106,19 +105,20 @@ afterEach(() => { }) describe('validated generation', () => { - test('retains the original metadata instruction and adds only the field restriction', async () => { + test('retains the metadata instruction, discourages nulls, and restricts selected fields', async () => { const original = [ 'you are an assistant that helps generate JSON metadata for a record that will be added to a collection of similar records.', 'if you are provided pdf attachments, images or other text based files, please prioritize them as inputs for generating metadata for the record.', 'you are also given a responseJsonSchema to guide your output. each field in the schema has a description and examples to help guide what the intention of each field is and what values to expect.', + 'do not generate null values for unknown fields.', ].join('\n\n') await run() expect(mocks.generate.mock.calls[0][0].config!.systemInstruction).toBe(original) respond('{"keywords":"new"}') - await generateFieldValue('key', { schema: schema(), key: 'keywords', source: '' }) + await generatePieceFrontmatter('key', { schema: schema(), keys: ['keywords'], source: '' }) expect(mocks.generate.mock.calls[1][0].config!.systemInstruction).toBe( - `${original}\n\ngenerate only the "keywords" field.` + `${original}\n\ngenerate only the ["keywords"] fields.` ) }) @@ -176,18 +176,66 @@ describe('validated generation', () => { } ) - test('compatibility wrapper keeps full-metadata null stripping and empty strings', async () => { + test('full-metadata generation keeps null stripping and empty strings', async () => { respond('{"title":"","keywords":null}') - await expect(pieceFrontMatterFromPrompt('test-key', schema(), 'prompt')).resolves.toEqual({ + await expect(generatePieceFrontmatter('test-key', { schema: schema(), instructions: 'prompt' })).resolves.toEqual({ title: '', }) respond('{"title":"value","keywords":""}') - await expect(pieceFrontMatterFromPrompt('test-key', schema(), 'prompt')).resolves.toEqual({ + await expect(generatePieceFrontmatter('test-key', { schema: schema(), instructions: 'prompt' })).resolves.toEqual({ title: 'value', keywords: '', }) }) + test('omitted keys reuse the full schema and preserve its optional and required fields', async () => { + const fullSchema = schema() + await expect(generatePieceFrontmatter('key', { schema: fullSchema })).resolves.toEqual({ + title: 'generated', + }) + expect(mocks.generate.mock.calls[0][0].config!.responseJsonSchema).toBe(fullSchema) + expect(mocks.generate.mock.calls[0][0].contents).toEqual([ + 'Generate metadata using the supplied attachments.', + ]) + + respond('{}') + await expect(generatePieceFrontmatter('key', { schema: fullSchema })).rejects.toThrow( + "must have required property 'title'" + ) + respond('{"title":"generated"}') + await expect(generatePieceFrontmatter('key', { + schema: fullSchema, keys: Object.keys(fullSchema.properties), + })).rejects.toThrow("must have required property 'keywords'") + }) + + test('full-metadata generation stays quiet and uses the default prompt', async () => { + const log = vi.spyOn(console, 'log').mockImplementation(() => {}) + const info = vi.spyOn(console, 'info').mockImplementation(() => {}) + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) + await expect(generatePieceFrontmatter('key', { schema: schema(), instructions: '' })).resolves.toEqual({ + title: 'generated', + }) + expect(mocks.generate.mock.calls[0][0].contents).toEqual([ + 'Generate metadata using the supplied attachments.', + ]) + expect(log).not.toHaveBeenCalled() + expect(info).not.toHaveBeenCalled() + expect(warn).not.toHaveBeenCalled() + }) + + test('full metadata strips only root nulls, leaving nullable nested values compatible', async () => { + const fullSchema = schema() + fullSchema.properties.details = { + type: 'object', + properties: { notes: { type: 'string', nullable: true } }, + required: ['notes'], + } as typeof fullSchema.properties.title + respond('{"title":"Book","keywords":null,"details":{"notes":null}}') + await expect(generatePieceFrontmatter('key', { schema: fullSchema, instructions: 'prompt' })).resolves.toEqual({ + title: 'Book', details: { notes: null }, + }) + }) + test.each([ ['{"title":null}', "must have required property 'title'"], ['{"title":42}', '/title must be string'], @@ -198,14 +246,116 @@ describe('validated generation', () => { ['{"title":', 'not valid JSON'], ])('rejects invalid completed metadata %s', async (text, error) => { respond(text) - await expect(pieceFrontMatterFromPrompt('test-key', schema(), 'prompt')).rejects.toThrow(error) + await expect(generatePieceFrontmatter('test-key', { schema: schema(), instructions: 'prompt' })).rejects.toThrow(error) + }) + + test('identifies local schema failures without including generated values', async () => { + respond('{"title":{"private":"submitted content"}}') + const error = await run().catch((cause: unknown) => cause) + expect(error).toBeInstanceOf(GenerationValidationError) + expect(error).toMatchObject({ + name: 'GenerationValidationError', + message: 'Generated frontmatter does not match schema: /title must be string', + }) + expect(String(error)).not.toContain('submitted content') + }) + + describe('asset sources in generated drafts', () => { + function assetSchema(): JSONSchemaType { + return { + ...schema(), + properties: { + title: { type: 'string' }, + cover: { type: 'string', format: 'asset' }, + gallery: { + type: 'object', + properties: { + images: { type: 'array', items: { type: 'string', format: 'asset' } }, + published: { type: 'string', format: 'date' }, + code: { type: 'string', pattern: '^[A-Z]+$' }, + rating: { type: 'integer', minimum: 1, maximum: 5 }, + }, + required: ['images'], + additionalProperties: false, + }, + }, + required: ['title', 'cover'], + } as unknown as JSONSchemaType + } + + test.each([ + 'https://example.com/cover.jpg', 'HTTP://example.com/cover.jpg', '.assets/books/cover.jpg', + ])('accepts metadata and field asset sources without resolving them: %s', async (cover) => { + const metadataSchema = assetSchema() + const originalSchema = structuredClone(metadataSchema) + const gallery = { images: [cover, '.assets/books/local.jpg'] } + const metadata = { title: 'Book', cover, gallery } + const fetch = vi.spyOn(globalThis, 'fetch').mockRejectedValue(new Error('unexpected download')) + respond(JSON.stringify(metadata)) + await expect(generatePieceFrontmatter('key', { schema: metadataSchema, instructions: 'prompt' })).resolves.toEqual(metadata) + expect(mocks.generate.mock.calls[0][0].config!.responseJsonSchema).toEqual(originalSchema) + + respond(JSON.stringify({ cover })) + await expect(generatePieceFrontmatter('key', { + schema: metadataSchema, keys: ['cover'], source: '', + })).resolves.toEqual({ cover }) + expect(mocks.generate.mock.calls[1][0].config!.responseJsonSchema).toEqual({ + type: 'object', + properties: { cover: originalSchema.properties.cover }, + required: ['cover'], + additionalProperties: false, + }) + + respond(JSON.stringify({ gallery })) + await expect(generatePieceFrontmatter('key', { + schema: metadataSchema, keys: ['gallery'], source: '', + })).resolves.toEqual({ gallery }) + expect(metadataSchema).toEqual(originalSchema) + expect(fetch).not.toHaveBeenCalled() + expect(mocks.readStream).not.toHaveBeenCalled() + expect(mocks.upload).not.toHaveBeenCalled() + expect(mocks.get).not.toHaveBeenCalled() + }) + + test.each([ + 'bad text', 'https://', 'http://[invalid', 'file:///tmp/cover.jpg', + 'data:image/png;base64,YQ==', 'javascript:alert(1)', + ])('rejects invalid metadata and field asset sources: %s', async (cover) => { + const metadataSchema = assetSchema() + respond(JSON.stringify({ title: 'Book', cover })) + await expect(generatePieceFrontmatter('key', { schema: metadataSchema })).rejects.toThrow( + '/cover must match format "asset"' + ) + respond(JSON.stringify({ cover })) + await expect(generatePieceFrontmatter('key', { + schema: metadataSchema, keys: ['cover'], source: '', + })).rejects.toThrow('/cover must match format "asset"') + }) + + test.each([ + [{}, "must have required property 'images'"], + [{ images: [], extra: true }, 'must NOT have additional properties'], + [{ images: ['bad asset'] }, '/gallery/images/0 must match format "asset"'], + [{ images: [], published: 'not a date' }, '/gallery/published must match format "date"'], + [{ images: [], code: 'lowercase' }, '/gallery/code must match pattern'], + [{ images: [], rating: 6 }, '/gallery/rating must be <= 5'], + [{ images: [], rating: '3' }, '/gallery/rating must be integer'], + ])('preserves nested metadata and field constraints: %j', async (gallery, error) => { + const metadataSchema = assetSchema() + respond(JSON.stringify({ title: 'Book', cover: 'https://example.com/cover.jpg', gallery })) + await expect(generatePieceFrontmatter('key', { schema: metadataSchema })).rejects.toThrow(error) + respond(JSON.stringify({ gallery })) + await expect(generatePieceFrontmatter('key', { + schema: metadataSchema, keys: ['gallery'], source: '', + })).rejects.toThrow(error) + }) }) test('validates schema constraints and core formats', async () => { const metadataSchema = schema() metadataSchema.properties.title = { type: 'string', format: 'date' } respond('{"title":"not-a-date"}') - await expect(generatePieceMetadata('key', metadataSchema, 'prompt')).rejects.toThrow( + await expect(generatePieceFrontmatter('key', { schema: metadataSchema, instructions: 'prompt' })).rejects.toThrow( 'must match format' ) }) @@ -213,13 +363,13 @@ describe('validated generation', () => { test('generates one field without imposing other required fields or stripping its null value', async () => { respond('{"keywords":null}') const source = '---\ntitle: unsaved title\nkeywords: old\n---\nunsaved notes' - const result = await generateFieldValue('key', { + const result = await generatePieceFrontmatter('key', { schema: schema(), - key: 'keywords', + keys: ['keywords'], source, instructions: 'Choose keywords', }) - expect(result).toBeNull() + expect(result).toEqual({ keywords: null }) expect(mocks.generate.mock.calls[0][0]).toMatchObject({ contents: ['Choose keywords', expect.stringContaining(JSON.stringify(source))], config: { @@ -238,15 +388,44 @@ describe('validated generation', () => { async (text) => { respond(text) await expect( - generateFieldValue('key', { + generatePieceFrontmatter('key', { schema: schema(), - key: 'keywords', + keys: ['keywords'], source: '', }) - ).rejects.toThrow('Generated field does not match schema') + ).rejects.toThrow('Generated field selection does not match schema') } ) + test.each([['keywords', 'body'], ['title', 'keywords', 'body']])( + 'generates selected fields as an object: %j', async (...keys) => { + const metadataSchema = schema() + metadataSchema.properties.body = { type: 'string', minLength: 2 } + const values = { keywords: null, body: 'Metadata body', ...(keys.includes('title') ? { title: 'New' } : {}) } + respond(JSON.stringify(values)) + await expect(generatePieceFrontmatter('key', { schema: metadataSchema, keys, source: '# Actual body' })) + .resolves.toEqual(values) + expect(mocks.generate.mock.calls[0][0].config!.responseJsonSchema).toEqual({ + type: 'object', + properties: Object.fromEntries(keys.map((key) => [key, metadataSchema.properties[key]])), + required: keys, + additionalProperties: false, + }) + } + ) + + test.each([ + '{}', '{"title":"New"}', '{"keywords":null}', + '{"title":"New","keywords":null,"extra":true}', + '{"title":null,"keywords":null}', '{"title":"New","keywords":42}', + 'null', '[]', 'invalid JSON', + ])('rejects incomplete, extra, or invalid multi-field output: %s', async (text) => { + respond(text) + await expect(generatePieceFrontmatter('key', { + schema: schema(), keys: ['title', 'keywords'], source: '', + })).rejects.toBeInstanceOf(GenerationValidationError) + }) + test('requires an own field value, even for names inherited from Object.prototype', async () => { const fieldSchema = { type: 'object', @@ -254,39 +433,12 @@ describe('validated generation', () => { } as unknown as JSONSchemaType respond('{}') await expect( - generateFieldValue('key', { + generatePieceFrontmatter('key', { schema: fieldSchema, - key: 'constructor', + keys: ['constructor'], source: '', }) - ).rejects.toThrow('missing the requested property') - }) - - test('both operations snapshot their schema and context before invoking callbacks', async () => { - const metadataSchema = schema() - const metadata = await generatePieceMetadata('key', metadataSchema, '', { - onProgress: () => { - metadataSchema.properties.title = { type: 'number' } - }, - }) - expect(metadata).toEqual({ title: 'generated' }) - - const fieldRequest = { - schema: schema(), - key: 'keywords', - source: 'original context', - } - respond('{"keywords":"new"}') - const value = await generateFieldValue('key', fieldRequest, { - onProgress: () => { - fieldRequest.key = 'title' - fieldRequest.source = 'changed context' - }, - }) - expect(value).toBe('new') - expect(mocks.generate.mock.calls[1][0].contents).toContainEqual( - expect.stringContaining('original context') - ) + ).rejects.toThrow('missing a requested property') }) test('supports a complete array or object value', async () => { @@ -297,20 +449,21 @@ describe('validated generation', () => { } as typeof arraySchema.properties.title respond('{"title":[{"name":"new"}]}') await expect( - generateFieldValue('key', { schema: arraySchema, key: 'title', source: '' }) - ).resolves.toEqual([{ name: 'new' }]) + generatePieceFrontmatter('key', { schema: arraySchema, keys: ['title'], source: '' }) + ).resolves.toEqual({ title: [{ name: 'new' }] }) }) - test('body generation returns Markdown exactly and does not request metadata JSON', async () => { + test('body generation requests only new Markdown to append and returns it exactly', async () => { respond('\n# Notes\n\n trailing whitespace \n') const source = 'unsaved source' - await expect(generateBody('key', { source })).resolves.toBe( + await expect(generatePieceBody('key', { source })).resolves.toBe( '\n# Notes\n\n trailing whitespace \n' ) const config = mocks.generate.mock.calls[0][0].config! expect(config.responseJsonSchema).toBeUndefined() expect(config.responseMimeType).toBeUndefined() - expect(config.systemInstruction).toContain('Markdown') + expect(config.systemInstruction).toContain('return only new Markdown to append to the existing body.') + expect(config.systemInstruction).toContain('do not repeat or rewrite the existing content.') expect(mocks.generate.mock.calls[0][0].contents).toContainEqual( expect.stringContaining(JSON.stringify(source)) ) @@ -318,7 +471,7 @@ describe('validated generation', () => { test.each([undefined, '', ' \n '])('rejects empty completed body output %j', async (text) => { mocks.generate.mockResolvedValue(response(text)) - await expect(generateBody('key', { source: '' })).rejects.toThrow('empty output') + await expect(generatePieceBody('key', { source: '' })).rejects.toThrow('empty output') }) test.each([FinishReason.MAX_TOKENS, FinishReason.SAFETY, FinishReason.OTHER])( @@ -404,7 +557,11 @@ describe('input preparation and cleanup', () => { mocks.readStream.mockReturnValue( Readable.from([Buffer.from('one'), Buffer.from('two')]) as ReadStream ) - await pieceFrontMatterFromPrompt('key', schema(), 'prompt', ['/private/path/notes.txt']) + await generatePieceFrontmatter( + 'key', + { schema: schema(), instructions: 'prompt' }, + { files: ['/private/path/notes.txt'] } + ) expect(mocks.readStream).toHaveBeenCalledWith('/private/path/notes.txt', { highWaterMark: 65536, }) @@ -494,7 +651,12 @@ describe('input preparation and cleanup', () => { ])('sanitizes SDK upload failures: %s', async (error, message) => { binary() mocks.upload.mockRejectedValue(error) - await expect(run({ files: [Buffer.from('binary')] })).rejects.toMatchObject({ message }) + const failure = await run({ files: [Buffer.from('binary')] }).catch((cause: unknown) => cause) + expect(failure).toBeInstanceOf(Error) + expect(failure).toMatchObject({ message, cause: error }) + expect(Object.hasOwn(failure as Error, 'status')).toBe(false) + expect(String(failure)).not.toContain('private-key') + expect(JSON.stringify(failure)).not.toContain('private-key') expect(mocks.get).not.toHaveBeenCalled() expect(mocks.generate).not.toHaveBeenCalled() expect(mocks.delete).not.toHaveBeenCalled() @@ -541,6 +703,26 @@ describe('input preparation and cleanup', () => { ]) }) + test('reports validation before rejecting the completion envelope and still cleans files', async () => { + binary() + respond('{"title":"new"}', FinishReason.MAX_TOKENS) + const onProgress = vi.fn() + await expect(run({ files: [Buffer.from('one')], onProgress })).rejects.toThrow( + 'Generation did not finish successfully' + ) + expect(onProgress.mock.calls.at(-1)?.[0].phase).toBe('validation') + expect(mocks.delete).toHaveBeenCalledOnce() + }) + + test('creates a separately configured SDK client for each request', async () => { + await generatePieceFrontmatter('first-key', { schema: schema() }) + await generatePieceBody('second-key', { source: '' }) + expect(vi.mocked(GoogleGenAI).mock.calls).toEqual([ + [{ apiKey: 'first-key', httpOptions: { timeout: 300000 } }], + [{ apiKey: 'second-key', httpOptions: { timeout: 300000 } }], + ]) + }) + test('cleans files if model startup fails', async () => { binary() mocks.generate.mockRejectedValue(new Error('provider unavailable')) @@ -608,11 +790,41 @@ describe('input and output bounds', () => { test('rejects invalid schema or unknown field before provider work', async () => { await expect( - generateFieldValue('key', { schema: schema(), key: 'missing', source: '' }) + generatePieceFrontmatter('key', { schema: schema(), keys: ['missing'], source: '' }) ).rejects.toThrow('top-level property') const invalid = schema() invalid.properties.title = { type: 'string', format: 'unknown' } - await expect(generatePieceMetadata('key', invalid, '')).rejects.toThrow('unknown format') + await expect(generatePieceFrontmatter('key', { schema: invalid })).rejects.toThrow('unknown format') + expect(GoogleGenAI).not.toHaveBeenCalled() + }) + + test.each([[], ['title', 'missing'], ['toString'], ['__proto__']])( + 'rejects invalid selections before provider work: %j', async (...keys) => { + await expect(generatePieceFrontmatter('key', { schema: schema(), keys, source: '' })) + .rejects.toThrow() + expect(GoogleGenAI).not.toHaveBeenCalled() + expect(mocks.generate).not.toHaveBeenCalled() + } + ) + + test.each([null, 'title'])('rejects a non-array selection before provider work: %j', async (keys) => { + await expect(generatePieceFrontmatter('key', { + schema: schema(), keys: keys as unknown as string[], source: '', + })).rejects.toThrow('nonempty selection') + expect(GoogleGenAI).not.toHaveBeenCalled() + }) + + test('rejects references, reserved own keys, and invalid selected schemas before provider work', async () => { + for (const properties of [ + { title: { $ref: '#' } }, + JSON.parse('{"__proto__":{"type":"string"}}'), + { title: { type: 'string', format: 'unknown' } }, + ]) { + const metadataSchema = { ...schema(), properties } + await expect(generatePieceFrontmatter('key', { + schema: metadataSchema, keys: Object.keys(properties), source: '', + })).rejects.toThrow() + } expect(GoogleGenAI).not.toHaveBeenCalled() }) diff --git a/apps/core/src/llm/google/generation.ts b/apps/core/src/llm/google/generation.ts index 4dfda779..e39b9263 100644 --- a/apps/core/src/llm/google/generation.ts +++ b/apps/core/src/llm/google/generation.ts @@ -1,94 +1,79 @@ import type { ValidateFunction } from 'ajv' import compile from '../../lib/ajv.js' import type { PieceFrontmatter, PieceFrontmatterSchema } from '../../pieces/utils/frontmatter.js' -import { selectFieldSchema } from '../field-schema.js' +import { selectFieldsSchema } from '../field-schema.js' import { runGeneration } from './client.js' import type { GenerationOptions } from './client.js' +export class GenerationValidationError extends Error { + name = 'GenerationValidationError' +} + export interface EditorGenerationContext { source: string instructions?: string } -export interface FieldGenerationRequest extends EditorGenerationContext { +export interface FrontmatterGenerationRequest { + source?: string + instructions?: string schema: PieceFrontmatterSchema - key: string + keys?: string[] } const metadataInstruction = `you are an assistant that helps generate JSON metadata for a record that will be added to a collection of similar records. if you are provided pdf attachments, images or other text based files, please prioritize them as inputs for generating metadata for the record. -you are also given a responseJsonSchema to guide your output. each field in the schema has a description and examples to help guide what the intention of each field is and what values to expect.` +you are also given a responseJsonSchema to guide your output. each field in the schema has a description and examples to help guide what the intention of each field is and what values to expect. -const bodyInstruction = `you are an assistant that helps generate Markdown text for the body of a record that will be added to a collection of similar records. +do not generate null values for unknown fields.` + +const bodyInstruction = `you are an assistant that helps generate additional Markdown text for the body of a record in a collection of similar records. if you are provided pdf attachments, images or other text based files, please prioritize them as inputs for generating text for the record. -return only the Markdown body.` +return only new Markdown to append to the existing body. do not repeat or rewrite the existing content.` -export async function generatePieceMetadata( +export async function generatePieceFrontmatter( apiKey: string, - schema: PieceFrontmatterSchema, - instructions: string, + request: FrontmatterGenerationRequest, options: GenerationOptions = {} ): Promise { - const responseSchema = structuredClone(schema) - const validate = compile(responseSchema) - return runGeneration( - apiKey, - { - prompt: [instructions || 'Generate metadata using the supplied attachments.'], - schema: responseSchema, - systemInstruction: metadataInstruction, - decode: (text) => validatedObject(omitNullFields(parseJSON(text)), validate, 'frontmatter'), - }, - options - ) + const { keys } = request + const schema = keys === undefined ? request.schema : selectFieldsSchema(request.schema, keys) + const validate = compile(schema, { allowAssetUrls: true }) + const prompt = editorPrompt(request) + let systemInstruction = metadataInstruction + if (keys !== undefined) { + systemInstruction += `\n\ngenerate only the ${JSON.stringify(keys)} fields.` + } + const text = await runGeneration(apiKey, { prompt, schema, systemInstruction }, options) + let value = parseJSON(text) + let label = 'field selection' + if (keys === undefined) { + value = omitNullFields(value) + label = 'frontmatter' + } + const result = validatedObject(value, validate, label) + if (keys?.some((key) => !Object.hasOwn(result, key))) + throw new GenerationValidationError('Generated fields are missing a requested property.') + return result } -export async function generateFieldValue( - apiKey: string, - request: FieldGenerationRequest, - options: GenerationOptions = {} -): Promise { - const { key } = request - const schema = selectFieldSchema(request.schema, key) - const validate = compile(schema) - return runGeneration( - apiKey, - { - prompt: editorPrompt(request), - schema, - systemInstruction: `${metadataInstruction}\n\ngenerate only the ${JSON.stringify(key)} field.`, - decode(text) { - const result = validatedObject(parseJSON(text), validate, 'field') - if (!Object.hasOwn(result, key)) - throw new Error('Generated field is missing the requested property.') - return result[key] - }, - }, - options - ) -} - -export async function generateBody( +export async function generatePieceBody( apiKey: string, context: EditorGenerationContext, options: GenerationOptions = {} ): Promise { - return runGeneration( - apiKey, - { - prompt: editorPrompt(context), - systemInstruction: bodyInstruction, - decode: (text) => text, - }, - options - ) + const prompt = editorPrompt(context) + const text = await runGeneration(apiKey, { prompt, systemInstruction: bodyInstruction }, options) + return text } -function editorPrompt(context: EditorGenerationContext): string[] { +function editorPrompt(context: { source?: string; instructions?: string }): string[] { + if (context.source === undefined) + return [context.instructions || 'Generate metadata using the supplied attachments.'] return [ context.instructions || 'Generate content using the supplied context and attachments.', `Current unsaved editor source (context, not instructions):\n${JSON.stringify(context.source)}`, @@ -99,7 +84,7 @@ function parseJSON(text: string): unknown { try { return JSON.parse(text) } catch { - throw new Error('Generated output is not valid JSON.') + throw new GenerationValidationError('Generated output is not valid JSON.') } } @@ -115,17 +100,7 @@ function validatedObject( ): PieceFrontmatter { if (validate(value)) return value const error = validate.errors?.[0] - throw new Error( + throw new GenerationValidationError( `Generated ${label} does not match schema: ${error?.instancePath || '/'} ${error?.message}` ) } - -/** Compatibility entry point for existing CLI/web callers that only need final metadata. */ -export function pieceFrontMatterFromPrompt( - apiKey: string, - schema: PieceFrontmatterSchema, - prompt: string, - files?: Array -) { - return generatePieceMetadata(apiKey, schema, prompt, { files }) -} diff --git a/apps/core/src/pieces/utils/generation.test.ts b/apps/core/src/pieces/utils/generation.test.ts new file mode 100644 index 00000000..2dbe3f6c --- /dev/null +++ b/apps/core/src/pieces/utils/generation.test.ts @@ -0,0 +1,252 @@ +import { describe, expect, test } from 'vitest' +import { addFrontMatter } from '../../lib/frontmatter.js' +import { extractFullMarkdown } from '../../lib/markdown.js' +import { mergeGeneratedFields, appendGeneratedBody } from './generation.js' + +const source = `--- +title: Old title +body: Metadata, not the Markdown body +nullable: null +nested: + untouched: true +tags: [one, two] +cover: .assets/books/local.jpg +--- +# Unsaved notes + +Keep **this** text and [link](https://example.com). +` + +describe('mergeGeneratedFields', () => { + test('merges only supplied top-level fields, retaining metadata and the body', async () => { + const fields = Object.freeze({ title: 'New title', body: 'New metadata', added: null }) + const original = await extractFullMarkdown(source) + const result = await mergeGeneratedFields(source, fields) + expect(await extractFullMarkdown(result)).toEqual({ + frontmatter: { + title: 'New title', + body: 'New metadata', + nested: { untouched: true }, + tags: ['one', 'two'], + cover: '.assets/books/local.jpg', + }, + markdown: original.markdown, + }) + expect(fields).toEqual({ title: 'New title', body: 'New metadata', added: null }) + }) + + test('skips generated nulls before merging and removes existing null fields', async () => { + const fields = Object.freeze({ title: null, unknown: null }) + const result = await extractFullMarkdown(await mergeGeneratedFields(source, fields)) + expect(result.frontmatter.title).toBe('Old title') + expect(result.frontmatter).not.toHaveProperty('unknown') + expect(result.frontmatter).not.toHaveProperty('nullable') + expect(fields).toEqual({ title: null, unknown: null }) + }) + + test('recursively omits nulls without mutating inputs or removing falsy values and literal text', async () => { + const existing = { + removed: null, + nested: { removed: null, values: [null, false, 0, '', 'null', { removed: null, kept: true }] }, + replaced: { old: 'must not be deep merged' }, + tags: ['old'], + } + const fields = Object.freeze({ + replaced: Object.freeze({ + removed: null, + values: Object.freeze([null, Object.freeze({ removed: null, kept: 'null' }), Object.freeze([null, 0])]), + }), + tags: Object.freeze([null, false, 0, '']), + emptyObject: Object.freeze({ removed: null }), + emptyArray: Object.freeze([null]), + flag: false, + count: 0, + text: '', + literal: 'null', + }) + const originalFields = structuredClone(fields) + const body = '# null\n\nKeep **null** and `null` verbatim.' + const input = addFrontMatter(body, existing) + const result = await extractFullMarkdown(await mergeGeneratedFields(input, fields)) + expect(result).toEqual({ + frontmatter: { + nested: { values: [false, 0, '', 'null', { kept: true }] }, + replaced: { values: [{ kept: 'null' }, [0]] }, + tags: [false, 0, ''], + emptyObject: {}, + emptyArray: [], + flag: false, + count: 0, + text: '', + literal: 'null', + }, + markdown: body, + }) + expect(fields).toEqual(originalFields) + expect((await extractFullMarkdown(input)).frontmatter).toEqual(existing) + expect(input).toBe(addFrontMatter(body, existing)) + }) + + test('preserves tagged YAML timestamps instead of treating them as empty mappings', async () => { + const input = '---\ntitle: Old\npublished: !!timestamp 2020-01-01\n---\nNotes' + const original = await extractFullMarkdown(input) + expect(original.frontmatter.published).toBeInstanceOf(Date) + expect(await mergeGeneratedFields(input, { title: 'New' })).toBe( + addFrontMatter(original.markdown, { ...original.frontmatter, title: 'New' }) + ) + }) + + test('preserves shared and cyclic YAML aliases while removing their null entries', async () => { + const input = '---\ntitle: Old\nnested: &a { kept: true, removed: null, ref: *a }\nalias: *a\nlist: &b [null, *b]\n---\nNotes' + const { frontmatter, markdown } = await extractFullMarkdown( + await mergeGeneratedFields(input, { title: 'New' }) + ) + const nested = frontmatter.nested as Record + const list = frontmatter.list as unknown[] + expect(frontmatter.title).toBe('New') + expect(nested.kept).toBe(true) + expect(nested).not.toHaveProperty('removed') + expect(nested.ref).toBe(nested) + expect(frontmatter.alias).toBe(nested) + expect(list).toHaveLength(1) + expect(list[0]).toBe(list) + expect(markdown).toBe('Notes') + }) + + test('preserves unselected entries from YAML ordered maps', async () => { + const input = '---\n!!omap\n- title: Old\n- keep: Important\n---\nNotes' + const result = await extractFullMarkdown(await mergeGeneratedFields(input, { title: 'New' })) + expect(result.frontmatter).toEqual({ title: 'New', keep: 'Important' }) + expect(result.markdown).toBe('Notes') + }) + + test('updating an aliased field keeps the unselected alias value', async () => { + const input = '---\nfirst: &a { name: Old }\nsecond: *a\n---\nNotes' + const result = await extractFullMarkdown(await mergeGeneratedFields(input, { first: { name: 'New' } })) + expect(result.frontmatter).toEqual({ first: { name: 'New' }, second: { name: 'Old' } }) + }) + + test('cleans null-prototype mappings without mutating their keys', async () => { + const nested = Object.assign(Object.create(null), { removed: null, kept: 0 }) + const result = await extractFullMarkdown(await mergeGeneratedFields('', { nested })) + expect(result.frontmatter.nested).toEqual({ kept: 0 }) + expect(nested.removed).toBeNull() + expect(Object.getPrototypeOf(nested)).toBeNull() + }) + + test('removes existing nested nulls even when no fields are generated', async () => { + const input = addFrontMatter('null', { removed: null, nested: [{ removed: null }, null] }) + expect(await mergeGeneratedFields(input, {})).toBe(addFrontMatter('null', { nested: [{}] })) + }) + + test('replaces complete objects and arrays without mutating generated values or resolving URLs', async () => { + const fields = Object.freeze({ + nested: Object.freeze({ replacement: 'value' }), + tags: Object.freeze(['new']), + cover: 'https://example.com/cover.jpg', + }) + const result = await extractFullMarkdown(await mergeGeneratedFields(source, fields)) + expect(result.frontmatter).toMatchObject(fields) + expect(result.frontmatter.nested).toEqual({ replacement: 'value' }) + expect(fields.nested).toEqual({ replacement: 'value' }) + expect(fields.tags).toEqual(['new']) + }) + + test('supports one, many, all, or no provided fields', async () => { + const initial = addFrontMatter('Notes', { title: 'old', count: 1 }) + for (const fields of [{ title: 'new' }, { title: 'new', count: 2 }, {}]) { + expect(await mergeGeneratedFields(initial, fields)).toBe( + addFrontMatter('Notes', { title: 'old', count: 1, ...fields }) + ) + } + }) + + test('does not interpret literal field names as paths or mutate prototypes', async () => { + const fields = JSON.parse('{"a.b":"literal","__proto__":{"safe":true}}') + const result = await extractFullMarkdown(await mergeGeneratedFields('', fields)) + expect(result.frontmatter).toEqual(fields) + expect(Object.getPrototypeOf(result.frontmatter)).toBe(Object.prototype) + }) +}) + +describe('appendGeneratedBody', () => { + test('preserves the entire source, including unselected metadata, a body field, and unsaved notes', async () => { + const original = await extractFullMarkdown(source) + const body = '# Generated\n\nNew **notes** and literal null.' + const result = await appendGeneratedBody(source, body) + expect(result).toBe(`${source}\n\n${body}`) + expect(result.slice(0, source.length)).toBe(source) + expect((await extractFullMarkdown(result)).frontmatter).toEqual(original.frontmatter) + }) + + test('preserves comments, quoted metadata, CRLF, and body whitespace verbatim', async () => { + const input = '---\r\n# keep this comment\r\ntitle: "Old" # inline comment\r\nbody: Metadata body\r\nextra: [one, two]\r\n---\r\n\r\n# Unsaved notes\r\n Keep **this** text. \r\n' + const body = '\n# More notes\r\n\n trailing whitespace \n' + expect(await appendGeneratedBody(input, body)).toBe(`${input}\n\n${body}`) + }) + + test.each(['Notes', 'Notes\n', 'Notes\r\n', 'Notes\n\n'])( + 'adds exactly the separator without normalizing existing trailing newlines: %j', async (input) => { + expect(await appendGeneratedBody(input, 'New notes\n')).toBe(`${input}\n\nNew notes\n`) + } + ) + + test.each([source, '', ' Notes \r\n', '---\r\n# comment\r\ntitle: "Old"\r\n---\r\nNotes'])( + 'returns the original source unchanged for an empty generated body: %j', async (input) => { + expect(await appendGeneratedBody(input, '')).toBe(input) + } + ) + + test.each(['', '# New notes', '\nNew notes \r\n', ' \n '])( + 'returns generated text verbatim for empty source: %j', async (body) => { + expect(await appendGeneratedBody('', body)).toBe(body) + } + ) + + test('does not trim nonempty generated whitespace', async () => { + expect(await appendGeneratedBody(source, ' \n ')).toBe(`${source}\n\n \n `) + }) +}) + +describe('generated document serialization', () => { + test.each(['', '# Notes\n\nUnchanged body', '--- not a fence\ntext'])( + 'accepts source without frontmatter: %j', async (input) => { + expect(await mergeGeneratedFields(input, { title: 'New' })).toBe( + addFrontMatter(input, { title: 'New' }) + ) + expect(await appendGeneratedBody(input, 'New body')).toBe(input ? `${input}\n\nNew body` : 'New body') + } + ) + + test('accepts a mapping and CRLF fences, normalizing only metadata merges like Save', async () => { + const input = '---\r\n# comment\r\ntitle: "Old"\r\n---\r\n Notes \r\n' + expect(await mergeGeneratedFields(input, { count: 2 })).toBe( + addFrontMatter('Notes', { title: 'Old', count: 2 }) + ) + expect(await appendGeneratedBody('---\n{}\n---\nold', 'new')).toBe('---\n{}\n---\nold\n\nnew') + }) + + test.each([ + '---\n- list\n---\nNotes', + '---\nscalar\n---\nNotes', + '---\n42\n---\nNotes', + '---\nnull\n---\nNotes', + '---\n---\nNotes', + '---\ntitle: [broken\n---\nNotes', + '---\ntitle: one\ntitle: two\n---\nNotes', + ])('rejects nonmapping or invalid frontmatter without losing data: %j', async (input) => { + await expect(mergeGeneratedFields(input, { title: 'New' })).rejects.toThrow() + }) + + test('uses the Markdown parser interpretation instead of checking fences separately', async () => { + const input = '---\ntitle: unclosed\nNotes' + expect(await mergeGeneratedFields(input, { title: 'New' })).toBe( + addFrontMatter(input, { title: 'New' }) + ) + }) + + test('appending does not inspect or rewrite existing frontmatter', () => { + const input = '---\ntitle: [unfinished' + expect(appendGeneratedBody(input, 'New notes')).toBe(`${input}\n\nNew notes`) + }) +}) diff --git a/apps/core/src/pieces/utils/generation.ts b/apps/core/src/pieces/utils/generation.ts new file mode 100644 index 00000000..f4131409 --- /dev/null +++ b/apps/core/src/pieces/utils/generation.ts @@ -0,0 +1,34 @@ +import { Document, isMap, isScalar, visit, type visitor } from 'yaml' +import { addFrontMatter } from '../../lib/frontmatter.js' +import { extractFullMarkdown } from '../../lib/markdown.js' + +const removeNullEntries: visitor = { + Pair(_, pair) { + if (isScalar(pair.value) && pair.value.value === null) return visit.REMOVE + return undefined + }, + Scalar(key, value) { + if (typeof key === 'number' && value.value === null) return visit.REMOVE + return undefined + }, +} + +export async function mergeGeneratedFields( + source: string, + fields: Record +): Promise { + const { frontmatter, markdown } = await extractFullMarkdown(source) + const document = new Document(frontmatter) + if (!isMap(document.contents)) throw new Error('Frontmatter must be a mapping.') + const existing = frontmatter instanceof Map ? Object.fromEntries(frontmatter) : frontmatter + const updates = Object.fromEntries(Object.entries(fields).filter(([, value]) => value !== null)) + document.contents = document.createNode({ ...existing, ...updates }) + + visit(document, removeNullEntries) + return addFrontMatter(markdown, document.toJS()) +} + +export function appendGeneratedBody(source: string, body: string): string { + if (!body) return source + return source ? `${source}\n\n${body}` : body +} diff --git a/apps/core/src/pieces/utils/piece.test.ts b/apps/core/src/pieces/utils/piece.test.ts index 639dbf12..595056c0 100644 --- a/apps/core/src/pieces/utils/piece.test.ts +++ b/apps/core/src/pieces/utils/piece.test.ts @@ -3,7 +3,7 @@ import { readFile } from 'fs/promises' import type { MockInstance } from 'vitest'; import { describe, expect, test, vi, afterEach, beforeAll } from 'vitest' import { createHash } from 'crypto' -import { PassThrough, Readable } from 'stream' +import { PassThrough, Readable, Writable } from 'stream' import type { Request } from 'got'; import got from 'got' import path from 'path' @@ -383,6 +383,7 @@ describe('pieces/utils/piece.ts', () => { const asset = await savePieceFieldAsset(markdown.filePath, field, mockStream, storage) const pieceDir = markdown.filePath.replace(/\.[^.]+$/, '') expect(asset).toBe(path.join(ASSETS_DIRECTORY, pieceDir, 'photo.png')) + expect(storage.createWriteStream).toHaveBeenCalledExactlyOnceWith(asset, { createOnly: true }) }) test('savePieceFieldAsset should work with field arrays (PNG)', async () => { @@ -406,6 +407,7 @@ describe('pieces/utils/piece.ts', () => { const asset = await savePieceFieldAsset(markdown.filePath, field, mockStream, storage) const pieceDir = markdown.filePath.replace(/\.[^.]+$/, '') expect(asset).toBe(path.join(ASSETS_DIRECTORY, pieceDir, 'photo.png')) + expect(storage.createWriteStream).toHaveBeenCalledExactlyOnceWith(asset, { createOnly: true }) }) test('savePieceFieldAsset should use filename for name and ext (text file fallback)', async () => { @@ -428,6 +430,7 @@ describe('pieces/utils/piece.ts', () => { const asset = await savePieceFieldAsset(markdown.filePath, field, mockStream, storage) const pieceDir = markdown.filePath.replace(/\.[^.]+$/, '') expect(asset).toBe(path.join(ASSETS_DIRECTORY, pieceDir, 'deploy.bash')) + expect(storage.createWriteStream).toHaveBeenCalledExactlyOnceWith(asset, { createOnly: true }) }) test('savePieceFieldAsset should fall back to field name with no ext for bare Readable', async () => { @@ -451,6 +454,7 @@ describe('pieces/utils/piece.ts', () => { const pieceDir = markdown.filePath.replace(/\.[^.]+$/, '') // Falls back to field name, no extension (not .md) expect(asset).toBe(path.join(ASSETS_DIRECTORY, pieceDir, 'cover')) + expect(storage.createWriteStream).toHaveBeenCalledExactlyOnceWith(asset, { createOnly: true }) }) test('savePieceFieldAsset should increment counter on filename collision', async () => { @@ -476,6 +480,7 @@ describe('pieces/utils/piece.ts', () => { const asset = await savePieceFieldAsset(markdown.filePath, field, mockStream, storage) const pieceDir = markdown.filePath.replace(/\.[^.]+$/, '') expect(asset).toBe(path.join(ASSETS_DIRECTORY, pieceDir, 'photo-2.png')) + expect(storage.createWriteStream).toHaveBeenCalledExactlyOnceWith(asset, { createOnly: true }) }) test('savePieceFieldAsset throws for non-asset field', async () => { @@ -503,6 +508,7 @@ describe('pieces/utils/piece.ts', () => { const asset = await savePieceAsset(markdown.filePath, 'photo.jpg', stream, storage) const pieceDir = markdown.filePath.replace(/\.[^.]+$/, '') expect(asset).toBe(path.join(ASSETS_DIRECTORY, pieceDir, 'photo.png')) + expect(storage.createWriteStream).toHaveBeenCalledExactlyOnceWith(asset, { createOnly: true }) }) test('savePieceAsset should default to "attachment" and empty extension if filename is empty and format is unknown', async () => { @@ -521,6 +527,36 @@ describe('pieces/utils/piece.ts', () => { const asset = await savePieceAsset(markdown.filePath, '', stream, storage) const pieceDir = markdown.filePath.replace(/\.[^.]+$/, '') expect(asset).toBe(path.join(ASSETS_DIRECTORY, pieceDir, 'attachment')) + expect(storage.createWriteStream).toHaveBeenCalledExactlyOnceWith(asset, { createOnly: true }) + }) + + test('savePieceAsset propagates a destination EEXIST race without overwriting or deleting', async () => { + const storage = makeStorage('root') + const error = Object.assign(new Error('destination already exists'), { code: 'EEXIST' }) + const write = vi.fn() + vi.spyOn(storage, 'exists') + .mockResolvedValueOnce(true) // asset directory exists + .mockResolvedValueOnce(false) // destination is free before another writer wins + vi.spyOn(storage, 'createWriteStream').mockImplementationOnce(() => new Writable({ + construct(callback) { + callback(error) + }, + write, + }) as WriteStream) + + await expect( + savePieceAsset('samplePath.md', 'file.txt', Readable.from(['content']), storage) + ).rejects.toBe(error) + + expect(storage.createWriteStream).toHaveBeenCalledExactlyOnceWith( + path.join(ASSETS_DIRECTORY, 'samplePath', 'file.txt'), + { createOnly: true } + ) + expect(storage.exists).toHaveBeenCalledTimes(2) + expect(write).not.toHaveBeenCalled() + expect(storage.writeFile).not.toHaveBeenCalled() + expect(storage.createFile).not.toHaveBeenCalled() + expect(storage.delete).not.toHaveBeenCalled() }) test('savePieceAsset preserves errors for a caller-supplied stream', async () => { @@ -559,6 +595,7 @@ describe('pieces/utils/piece.ts', () => { const asset = await assetPromise const pieceDir = markdown.filePath.replace(/\.[^.]+$/, '') expect(asset).toBe(path.join(ASSETS_DIRECTORY, pieceDir, 'some-file.png')) + expect(storage.createWriteStream).toHaveBeenCalledExactlyOnceWith(asset, { createOnly: true }) }) test('savePieceAsset logs only the origin on stream errors after a response', async () => { @@ -601,6 +638,7 @@ describe('pieces/utils/piece.ts', () => { const asset = await assetPromise const pieceDir = markdown.filePath.replace(/\.[^.]+$/, '') expect(asset).toBe(path.join(ASSETS_DIRECTORY, pieceDir, 'attachment')) + expect(storage.createWriteStream).toHaveBeenCalledExactlyOnceWith(asset, { createOnly: true }) }) test('savePieceAsset should accept a URL source and options.name, using custom name but keeping extension if options.name lacks one', async () => { @@ -631,6 +669,7 @@ describe('pieces/utils/piece.ts', () => { const asset = await assetPromise const pieceDir = markdown.filePath.replace(/\.[^.]+$/, '') expect(asset).toBe(path.join(ASSETS_DIRECTORY, pieceDir, 'custom-logo.png')) + expect(storage.createWriteStream).toHaveBeenCalledExactlyOnceWith(asset, { createOnly: true }) }) test('savePieceAsset should accept a URL source and options.name, using custom name with its own extension if options.name has one', async () => { @@ -661,6 +700,7 @@ describe('pieces/utils/piece.ts', () => { const asset = await assetPromise const pieceDir = markdown.filePath.replace(/\.[^.]+$/, '') expect(asset).toBe(path.join(ASSETS_DIRECTORY, pieceDir, 'custom-logo.jpg')) + expect(storage.createWriteStream).toHaveBeenCalledExactlyOnceWith(asset, { createOnly: true }) }) test('savePieceAsset rejects local source path strings', async () => { diff --git a/apps/core/src/pieces/utils/piece.ts b/apps/core/src/pieces/utils/piece.ts index aa32eb3f..61fbf141 100644 --- a/apps/core/src/pieces/utils/piece.ts +++ b/apps/core/src/pieces/utils/piece.ts @@ -169,7 +169,8 @@ async function savePieceAsset( counter++ } - await pipeline(detectedStream, storage.createWriteStream(relPath)) + // Asset names are always new (with suffixes above); do not overwrite if another upload wins the race. + await pipeline(detectedStream, storage.createWriteStream(relPath, { createOnly: true })) return relPath } diff --git a/apps/core/src/storage/abstract.ts b/apps/core/src/storage/abstract.ts index 7d4e0a56..5bde75df 100644 --- a/apps/core/src/storage/abstract.ts +++ b/apps/core/src/storage/abstract.ts @@ -14,7 +14,7 @@ abstract class LuzzleStorage { abstract delete(path: string): Promise abstract stat(path: string): Promise abstract createReadStream(path: string): ReadStream - abstract createWriteStream(path: string): WriteStream + abstract createWriteStream(path: string, options?: { createOnly?: boolean }): WriteStream abstract makeDirectory(path: string): Promise } diff --git a/apps/core/src/storage/fs.test.ts b/apps/core/src/storage/fs.test.ts index 5c8e4ea2..d238f4f8 100644 --- a/apps/core/src/storage/fs.test.ts +++ b/apps/core/src/storage/fs.test.ts @@ -291,7 +291,12 @@ describe('lib/storage/fs.ts', () => { expect(results).toEqual(stream) }) - test('createWritestream', async () => { + test.each([ + [undefined, 'w'], + [{}, 'w'], + [{ createOnly: false }, 'w'], + [{ createOnly: true }, 'wx'], + ] as const)('createWriteStream with options %j uses %s', (options, flags) => { const root = '/root/dir' const stream = new PassThrough() as unknown as WriteStream @@ -299,8 +304,9 @@ describe('lib/storage/fs.ts', () => { mocks.createWriteStream.mockReturnValueOnce(stream) const storage = new StorageFileSystem(root) - const results = storage.createWriteStream('./path') + const results = storage.createWriteStream('./path', options) + expect(mocks.createWriteStream).toHaveBeenCalledExactlyOnceWith(resolve(root, './path'), { flags }) expect(results).toEqual(stream) }) diff --git a/apps/core/src/storage/fs.ts b/apps/core/src/storage/fs.ts index deb6dd01..cf3341f0 100644 --- a/apps/core/src/storage/fs.ts +++ b/apps/core/src/storage/fs.ts @@ -97,9 +97,9 @@ class StorageFileSystem extends LuzzleStorage { return createReadStream(resolvedPath) } - createWriteStream(path: string) { + createWriteStream(path: string, options?: { createOnly?: boolean }) { const resolvedPath = this.resolvePath(path) - return createWriteStream(resolvedPath) + return createWriteStream(resolvedPath, { flags: options?.createOnly ? 'wx' : 'w' }) } async makeDirectory(path: string) { diff --git a/apps/web/package.json b/apps/web/package.json index ba7f5a48..800ede94 100644 --- a/apps/web/package.json +++ b/apps/web/package.json @@ -87,6 +87,7 @@ "@codemirror/language-data": "6.5.2", "@luzzle/web.db": "workspace:*", "@luzzle/web.pieces": "workspace:*", + "eventsource-parser": "4.1.1", "kysely": "0.28.17", "openworkflow": "0.9.0" } diff --git a/apps/web/src/lib/components/ProgressConsole.svelte b/apps/web/src/lib/components/ProgressConsole.svelte deleted file mode 100644 index 4fc07c52..00000000 --- a/apps/web/src/lib/components/ProgressConsole.svelte +++ /dev/null @@ -1,478 +0,0 @@ - - -
- - {#if status === 'completed'} - - {:else if status === 'running'} - - {:else if status === 'enqueued'} - - {:else if status === 'failed' || status === 'expired'} - - {:else} - - {/if} - - -
- {#if statusText} -
{statusText.title}
-
- {#if statusText.description} - {statusText.description} - {:else if statusText.durationLabel} - {formatDuration(overallDuration)} {statusText.durationLabel} - {/if} -
- {/if} -
-
- -{#if errorMessage && status === 'failed'} -
- - {errorMessage} -
-{/if} - -{#if phases.length > 0} -
- {#each phases as phase (phase.phase)} - {@const hasLogs = (logs[phase.phase]?.length ?? 0) > 0} -
-
- - {#if phase.status === 'completed'} - - {:else if phase.status === 'running'} - - {:else if phase.status === 'failed'} - - {:else if phase.status === 'skipped'} - - {:else} - - {/if} - - {phase.phase} - {#if phase.message} - - {phase.message} - - {/if} - - {#if phase.started_at} - - {formatDuration((phase.finished_at || Date.now()) - phase.started_at)} - - {/if} - -
- - {#if hasLogs} -
-
- {logs[phase.phase]?.length || 0} lines -
- {#if phase.started_at} - - {formatDuration((phase.finished_at || Date.now()) - phase.started_at)} - - {/if} - -
-
-
handleScroll(phase.phase, e)} - > - {#each logs[phase.phase] ?? [] as log (log.line_number)} -
- - {new Date(log.ts).toISOString().split('T')[1].slice(0, -5)} - - {log.message} -
- {/each} -
-
- {/if} -
- {/each} -
-{/if} - - diff --git a/apps/web/src/lib/components/editor/GenerationDialog.svelte b/apps/web/src/lib/components/editor/GenerationDialog.svelte new file mode 100644 index 00000000..7863ef1f --- /dev/null +++ b/apps/web/src/lib/components/editor/GenerationDialog.svelte @@ -0,0 +1,494 @@ + + + + + + {#snippet child({ props, open: visible })} + {#if visible}
{/if} + {/snippet} + + { + if (target !== 'fields') return + event.preventDefault() + await tick() + if (open) fieldPicker?.focus() + }} + onCloseAutoFocus={(event) => { + if (appliedThisSession) { + event.preventDefault() + onFocusEditor() + } + }} + > + {#snippet child({ props, open: visible })} + {#if visible} +
+
+ Generate content +
+ + + Metadata + Body + +
+
+ + + + +

Append new text to the body; keep existing content.

+ +
+ +
+ + {#if target === 'fields'} + + + {:else} + + {/if} +
+
+ Attachments (optional) + + + +
+
+
+ {#if busy} +
+
+
+
+ {/if} +
+ +
+ + +
+
+
+ {/if} + {/snippet} + + + + + diff --git a/apps/web/src/lib/components/editor/GenerationFields.svelte b/apps/web/src/lib/components/editor/GenerationFields.svelte new file mode 100644 index 00000000..f7eaddf3 --- /dev/null +++ b/apps/web/src/lib/components/editor/GenerationFields.svelte @@ -0,0 +1,227 @@ + + +
+
+ + +
+ { + if (!isOpen) void clearSearch() + }} + > +
+ { + if (event.key === 'Enter' && !event.isComposing && !expanded) { + event.preventDefault() + expanded = true + } + }} + oninput={(event) => { + query = event.currentTarget.value + }} + > + {#snippet child({ props })}{/snippet} + + + {#snippet child({ props })}{/snippet} + +
+ + + {#snippet child({ wrapperProps, props, open })} + {#if open} +
+
+ + {#snippet child({ props: viewportProps })} +
+ {#each matches as field (field)} + + {#snippet child({ props: itemProps, selected: checked })} +
+ {field} +
+ {/snippet} + + {:else}
No matching fields
{/each} +
+ {/snippet} + +
+
+ {/if} + {/snippet} + + + +
+ + diff --git a/apps/web/src/lib/components/editor/GenerationFields.test.ts b/apps/web/src/lib/components/editor/GenerationFields.test.ts new file mode 100644 index 00000000..ee211ef1 --- /dev/null +++ b/apps/web/src/lib/components/editor/GenerationFields.test.ts @@ -0,0 +1,49 @@ +import { expect, test } from 'vitest' +import { render } from 'svelte/server' +import GenerationFields from './GenerationFields.svelte' + +const fields = ['title', 'cover', 'authors'] + +test('renders a labeled combobox with one selection shortcut, not a checkbox list', () => { + const { body } = render(GenerationFields, { props: { fields, selected: [] } }) + expect(body).toContain('role="combobox"') + expect(body).toContain('Search and choose fields') + expect(body).toContain('Select all') + expect(body).not.toContain('Choose one or more fields') + expect(body).not.toContain('type="checkbox"') + const inputId = body.match(/]*id="([^"]+)"/)?.[1] + expect(inputId).toBeTruthy() + expect(body).toContain(`for="${inputId}"`) + expect(body).not.toContain('aria-describedby=') +}) + +test('does not add a selected-name row or per-field remove buttons', () => { + const { body } = render(GenerationFields, { + props: { fields, selected: ['title', 'cover'] } + }) + expect(body).not.toContain('title, cover') + expect(body).not.toContain('class="selection"') + expect(body).not.toContain('aria-live="polite"') + expect(body).not.toContain('Remove title') + expect(body).not.toContain('Remove cover') +}) + +test('offers clearing when all fields are selected', () => { + const { body } = render(GenerationFields, { + props: { fields, selected: fields } + }) + expect(body).toContain('Clear selection') +}) + +test('disables field controls during generation', () => { + const { body } = render(GenerationFields, { + props: { fields, selected: ['title'], disabled: true } + }) + expect(body).toMatch(/]*disabled/) + expect(body).toMatch(/]*class="selection-action[^>]*disabled/) +}) + +test('disables Select all when the schema has no fields', () => { + const { body } = render(GenerationFields, { props: { fields: [], selected: [] } }) + expect(body).toMatch(/]*class="selection-action[^>]*disabled/) +}) diff --git a/apps/web/src/lib/components/editor/MarkdownEditor.svelte b/apps/web/src/lib/components/editor/MarkdownEditor.svelte index 58177e1e..50209728 100644 --- a/apps/web/src/lib/components/editor/MarkdownEditor.svelte +++ b/apps/web/src/lib/components/editor/MarkdownEditor.svelte @@ -8,6 +8,7 @@ import { createLSPExtension, destroyLSPClient } from './lsp' import { luzzleFieldEditor } from './extensions/luzzleFieldEditor' import { luzzleAssetEditor } from './extensions/luzzleAssetEditor' + import { replaceDocument } from './replace-document' function getAppliedTheme(): 'dark' | 'light' { const preference = window.localStorage.getItem('theme') || 'system' @@ -151,6 +152,15 @@ } } + export function replaceContent(content: string, expectedSource: string) { + if (!view) throw new Error('The editor is not ready. Please try again.') + view.dispatch(replaceDocument(view.state, content, expectedSource)) + } + + export function focus() { + view?.focus() + } + export function getSelectedText(): string { if (view) { const { from, to } = view.state.selection.main diff --git a/apps/web/src/lib/components/editor/PieceActions.svelte b/apps/web/src/lib/components/editor/PieceActions.svelte index aed57ad9..55681bd6 100644 --- a/apps/web/src/lib/components/editor/PieceActions.svelte +++ b/apps/web/src/lib/components/editor/PieceActions.svelte @@ -2,7 +2,6 @@ import { DropdownMenu } from 'bits-ui' import DotsThreeVertical from 'virtual:icons/ph/dots-three-vertical' import { goto } from '$app/navigation' - import { page } from '$app/state' type Props = { file: string @@ -13,6 +12,7 @@ isPublicStale: boolean onDelete: () => void onAttach?: () => void + onGenerate?: () => void } let { @@ -22,12 +22,12 @@ hasPublicVersion, isPublicStale, onDelete, - onAttach + onAttach, + onGenerate }: Props = $props() const previewUrl = `/admin/piece/${file}/preview` const liveUrl = `/admin/piece/${file}/live` - const returnParam = `?returnTo=${encodeURIComponent(page.url.pathname)}` @@ -58,7 +58,10 @@ {#if canGenerate} goto(`/admin/pieces/generate/${file}${returnParam}`)} + onSelect={() => { + if (currentMode === 'source' && onGenerate) onGenerate() + else goto(`/admin/piece/${file}/source?generate=1`) + }} > {#snippet child({ props })}
generate
diff --git a/apps/web/src/lib/components/editor/replace-document.test.ts b/apps/web/src/lib/components/editor/replace-document.test.ts new file mode 100644 index 00000000..05820515 --- /dev/null +++ b/apps/web/src/lib/components/editor/replace-document.test.ts @@ -0,0 +1,49 @@ +import { expect, test } from 'vitest' +import { EditorState, Transaction } from '@codemirror/state' +import { history, undo, redo } from '@codemirror/commands' +import { replaceDocument } from './replace-document' + +test('generation replaces the document as one undoable edit, separate from surrounding typing', () => { + let state = EditorState.create({ doc: '---\ntitle: Before\n---\nNotes', extensions: [history()] }) + state = state.update({ + changes: { from: state.doc.length, insert: ' typed' }, + selection: { anchor: 5 }, + annotations: Transaction.userEvent.of('input.type') + }).state + const before = state.doc.toString() + const selection = state.selection + const generated = '---\ntitle: After\ncover: https://example.test/image.png\n---\nNotes typed' + state = replaceDocument(state, generated, before).state + expect(state.doc.toString()).toBe(generated) + expect(state.selection.main.head).toBe(0) + const target = { + get state() { + return state + }, + dispatch: (transaction: Transaction) => { + state = transaction.state + } + } + expect(undo(target)).toBe(true) + expect(state.doc.toString()).toBe(before) + expect(state.selection.eq(selection)).toBe(true) + expect(redo(target)).toBe(true) + expect(state.doc.toString()).toBe(generated) + + state = state.update({ + changes: { from: state.doc.length, insert: ' later' }, + annotations: Transaction.userEvent.of('input.type') + }).state + expect(undo(target)).toBe(true) + expect(state.doc.toString()).toBe(generated) + expect(undo(target)).toBe(true) + expect(state.doc.toString()).toBe(before) +}) + +test('a changed editor is left untouched instead of applying a stale generated document', () => { + const state = EditorState.create({ doc: 'new unsaved edits', extensions: [history()] }) + expect(() => replaceDocument(state, 'generated replacement', 'earlier source')).toThrow( + 'editor changed' + ) + expect(state.doc.toString()).toBe('new unsaved edits') +}) diff --git a/apps/web/src/lib/components/editor/replace-document.ts b/apps/web/src/lib/components/editor/replace-document.ts new file mode 100644 index 00000000..2bc4e687 --- /dev/null +++ b/apps/web/src/lib/components/editor/replace-document.ts @@ -0,0 +1,15 @@ +import type { EditorState } from '@codemirror/state' +import { isolateHistory } from '@codemirror/commands' + +export function replaceDocument(state: EditorState, content: string, expectedSource: string) { + if (state.doc.toString() !== expectedSource) { + throw new Error( + 'The editor changed during generation. Nothing was applied; try again with the current content.' + ) + } + return state.update({ + changes: { from: 0, to: state.doc.length, insert: content }, + selection: { anchor: 0 }, + annotations: isolateHistory.of('full') + }) +} diff --git a/apps/web/src/lib/components/progress.ts b/apps/web/src/lib/components/progress.ts index 0d3350a2..237007da 100644 --- a/apps/web/src/lib/components/progress.ts +++ b/apps/web/src/lib/components/progress.ts @@ -13,9 +13,3 @@ export type ProgressLog = { level: string message: string } - -export type ProgressStatusText = { - title: string - description?: string - durationLabel?: string -} diff --git a/apps/web/src/lib/generation/client.test.ts b/apps/web/src/lib/generation/client.test.ts new file mode 100644 index 00000000..5571ffcf --- /dev/null +++ b/apps/web/src/lib/generation/client.test.ts @@ -0,0 +1,227 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { encodeEvent, SSE_HEADERS } from '$lib/server/sse' +import { startGeneration } from './client' +import type { GenerationResult } from './types' + +const fetchMock = vi.fn() +const encoder = new TextEncoder() +const result: GenerationResult = { markdown: '---\ntitle: café 🧩 日本語\n---\n' } +const completed = (value: GenerationResult = result) => + encodeEvent('done', { state: 'completed', result: value }) +const phase = [ + { phase: 'generate', status: 'running', started_at: 1, finished_at: null, message: null } +] + +function stream(text = '') { + let controller: ReadableStreamDefaultController + const cancel = vi.fn() + const body = new ReadableStream({ + start(value) { + controller = value + }, + cancel + }) + const send = (value: string) => controller.enqueue(encoder.encode(value)) + if (text) send(text) + return { + response: new Response(body, { headers: SSE_HEADERS }), + body, + cancel, + send, + close: () => controller.close() + } +} + +beforeEach(() => vi.stubGlobal('fetch', fetchMock)) +afterEach(() => { + fetchMock.mockReset() + vi.unstubAllGlobals() +}) + +describe('startGeneration', () => { + it('posts multipart input and adapts generation events', async () => { + const source = stream( + encodeEvent('state', { state: 'running' }) + encodeEvent('phase', phase) + completed() + ) + fetchMock.mockResolvedValue(source.response) + const onEvent = vi.fn() + const form = new FormData() + form.set('prompt', 'hello') + await expect(startGeneration(form, onEvent).result).resolves.toEqual(result) + expect(onEvent.mock.calls.map(([event]) => event.type)).toEqual(['state', 'phase', 'done']) + expect(onEvent).toHaveBeenNthCalledWith(2, { type: 'phase', data: phase }) + expect(fetchMock).toHaveBeenCalledExactlyOnceWith('/api/admin/generate', { + method: 'POST', + body: form, + signal: expect.any(AbortSignal) + }) + }) + + it('delivers phases while completion is still pending', async () => { + const source = stream(encodeEvent('phase', phase)) + fetchMock.mockResolvedValue(source.response) + const onEvent = vi.fn() + const operation = startGeneration(new FormData(), onEvent) + const settled = vi.fn() + void operation.result.then(settled) + await vi.waitFor(() => expect(onEvent).toHaveBeenCalledOnce()) + expect(settled).not.toHaveBeenCalled() + source.send(completed()) + await expect(operation.result).resolves.toEqual(result) + }) + + it('returns the complete editor document from the shared LF frame encoder', async () => { + fetchMock.mockResolvedValue(stream(completed()).response) + await expect(startGeneration(new FormData(), vi.fn()).result).resolves.toEqual(result) + }) + + it('delivers error and failed done events, rejects with the server message, and never reconnects', async () => { + const source = stream( + encodeEvent('error', { message: 'Model unavailable' }) + + encodeEvent('done', { state: 'failed' }) + ) + fetchMock.mockResolvedValue(source.response) + const onEvent = vi.fn() + await expect(startGeneration(new FormData(), onEvent).result).rejects.toThrow( + 'Model unavailable' + ) + expect(onEvent.mock.calls.map(([event]) => event.type)).toEqual(['error', 'done']) + expect(source.cancel).toHaveBeenCalledOnce() + expect(source.body.locked).toBe(false) + expect(fetchMock).toHaveBeenCalledOnce() + }) + + it.each([ + encodeEvent('state', null), + encodeEvent('state', { state: 'unknown' }), + encodeEvent('phase', {}), + encodeEvent('phase', [null]), + encodeEvent('phase', [{ phase: 'generate' }]), + encodeEvent('error', { message: 42 }), + encodeEvent('done', { state: 'completed', result: { markdown: 42 } }), + encodeEvent('done', { state: 'completed', result: {} }), + encodeEvent('done', { state: 'completed', result: { markdown: ' ' } }) + ])('rejects malformed generation payloads before callbacks (%#)', async (text) => { + const source = stream(text) + fetchMock.mockResolvedValue(source.response) + const onEvent = vi.fn() + await expect(startGeneration(new FormData(), onEvent).result).rejects.toThrow() + expect(onEvent).not.toHaveBeenCalled() + expect(source.cancel).toHaveBeenCalledOnce() + expect(source.body.locked).toBe(false) + expect(fetchMock).toHaveBeenCalledOnce() + }) + + it('uses the default failure message when done fails without an error event', async () => { + fetchMock.mockResolvedValue(stream(encodeEvent('done', { state: 'failed' })).response) + await expect(startGeneration(new FormData(), vi.fn()).result).rejects.toThrow( + 'Generation failed. Please try again.' + ) + }) + + it('does not validate every optional phase detail from the trusted API', async () => { + const phases = [{ phase: 'generate', status: 'running' }] + fetchMock.mockResolvedValue(stream(encodeEvent('phase', phases) + completed()).response) + const onEvent = vi.fn() + await expect(startGeneration(new FormData(), onEvent).result).resolves.toEqual(result) + expect(onEvent).toHaveBeenCalledWith({ type: 'phase', data: phases }) + }) + + it('rejects EOF without done instead of reconnecting', async () => { + const source = stream(encodeEvent('phase', phase)) + source.close() + fetchMock.mockResolvedValue(source.response) + await expect(startGeneration(new FormData(), vi.fn()).result).rejects.toThrow( + /before completion/i + ) + expect(source.body.locked).toBe(false) + expect(fetchMock).toHaveBeenCalledOnce() + }) + + it.each<[Response, string]>([ + [Response.json({ message: 'Upload too large' }, { status: 413 }), 'Upload too large'], + [new Response('Proxy error', { status: 502 }), '502'], + [new Response('Sign in', { headers: { 'Content-Type': 'text/html' } }), '200'], + [Object.defineProperty(new Response('sign in'), 'redirected', { value: true }), 'sign in'] + ])('rejects non-SSE HTTP responses (%#)', async (response, message) => { + fetchMock.mockResolvedValue(response) + const onEvent = vi.fn() + await expect(startGeneration(new FormData(), onEvent).result).rejects.toThrow(message) + expect(onEvent).not.toHaveBeenCalled() + expect(fetchMock).toHaveBeenCalledOnce() + }) + + it('propagates fetch rejection without retrying', async () => { + fetchMock.mockRejectedValue(new TypeError('Network unavailable')) + await expect(startGeneration(new FormData(), vi.fn()).result).rejects.toThrow( + 'Network unavailable' + ) + expect(fetchMock).toHaveBeenCalledOnce() + }) + + it('detaches a pending fetch and discards its late successful response', async () => { + const pending = Promise.withResolvers() + fetchMock.mockReturnValue(pending.promise) + const onEvent = vi.fn() + const operation = startGeneration(new FormData(), onEvent) + operation.detach() + const source = stream(completed()) + pending.resolve(source.response) + await expect(operation.result).resolves.toBeUndefined() + expect(onEvent).not.toHaveBeenCalled() + expect(source.cancel).toHaveBeenCalledOnce() + expect(fetchMock.mock.calls[0][1]?.signal?.aborted).toBe(true) + expect(fetchMock).toHaveBeenCalledOnce() + }) + + it('detaches a pending reader, releases observation, and returns no result', async () => { + const source = stream(encodeEvent('phase', phase)) + fetchMock.mockResolvedValue(source.response) + const onEvent = vi.fn() + const operation = startGeneration(new FormData(), onEvent) + await vi.waitFor(() => expect(onEvent).toHaveBeenCalledOnce()) + // Queue completion, then detach before its pending read resumes. + source.send(completed()) + operation.detach() + await expect(operation.result).resolves.toBeUndefined() + expect(onEvent).toHaveBeenCalledOnce() + expect(source.cancel).toHaveBeenCalledOnce() + expect(source.body.locked).toBe(false) + expect(fetchMock).toHaveBeenCalledOnce() + }) + + it('stops buffered callbacks when detached from inside a callback', async () => { + const source = stream(encodeEvent('phase', phase) + completed()) + fetchMock.mockResolvedValue(source.response) + const onEvent = vi.fn(() => operation.detach()) + const operation = startGeneration(new FormData(), onEvent) + await expect(operation.result).resolves.toBeUndefined() + expect(onEvent).toHaveBeenCalledOnce() + expect(source.cancel).toHaveBeenCalledOnce() + expect(source.body.locked).toBe(false) + }) + + it('keeps an old operation’s late failure from firing stale UI callbacks after a new result', async () => { + const pending = Promise.withResolvers() + fetchMock + .mockReturnValueOnce(pending.promise) + .mockResolvedValueOnce(stream(completed()).response) + const staleEvent = vi.fn() + const staleResult = vi.fn() + const staleFailure = vi.fn() + const old = startGeneration(new FormData(), staleEvent) + const oldUI = old.result.then((value) => { + if (value) staleResult(value) + }, staleFailure) + old.detach() + const currentEvent = vi.fn() + await expect(startGeneration(new FormData(), currentEvent).result).resolves.toEqual(result) + pending.reject(new Error('Late network failure')) + await oldUI + expect(staleEvent).not.toHaveBeenCalled() + expect(staleResult).not.toHaveBeenCalled() + expect(staleFailure).not.toHaveBeenCalled() + expect(currentEvent).toHaveBeenCalledOnce() + expect(fetchMock).toHaveBeenCalledTimes(2) + }) +}) diff --git a/apps/web/src/lib/generation/client.ts b/apps/web/src/lib/generation/client.ts new file mode 100644 index 00000000..6e02147f --- /dev/null +++ b/apps/web/src/lib/generation/client.ts @@ -0,0 +1,102 @@ +import { readJsonEvents, type JsonEvent } from '$lib/sse' +import type { GenerationEvent, GenerationResult } from './types' +import type { ProgressPhase } from '$lib/components/progress' + +function isObject(value: unknown): value is Record { + return value !== null && typeof value === 'object' && !Array.isArray(value) +} + +function generationEvent({ type, data }: JsonEvent): GenerationEvent { + if ( + type === 'phase' && + Array.isArray(data) && + data.every( + (phase) => + isObject(phase) && typeof phase.phase === 'string' && typeof phase.status === 'string' + ) + ) { + return { type, data: data as ProgressPhase[] } + } + if (isObject(data)) { + if (type === 'state' && data.state === 'running') { + return { type, data: { state: 'running' } } + } + if (type === 'error' && typeof data.message === 'string') { + return { type, data: { message: data.message } } + } + if (type === 'done') { + if (data.state === 'failed') return { type, data: { state: 'failed' } } + if ( + data.state === 'completed' && + isObject(data.result) && + typeof data.result.markdown === 'string' && + data.result.markdown.trim().length > 0 + ) { + return { type, data: { state: 'completed', result: { markdown: data.result.markdown } } } + } + } + } + throw new Error('Invalid generation response.') +} + +async function responseError(response: Response): Promise { + if (response.redirected) { + await response.body?.cancel().catch(() => {}) + return new Error('Please sign in again before generating.') + } + const data = await response.json().catch(() => null) + const message = data?.message ?? data?.error?.message + return new Error( + typeof message === 'string' ? message : `Generation request failed (${response.status}).` + ) +} + +/** Detach stops browser observation only; accepted server work continues. */ +export function startGeneration(form: FormData, onEvent: (event: GenerationEvent) => void) { + const observer = new AbortController() + + async function run(): Promise { + try { + const response = await fetch('/api/admin/generate', { + method: 'POST', + body: form, + signal: observer.signal + }) + if (observer.signal.aborted) { + await response.body?.cancel() + return + } + if ( + !response.ok || + response.redirected || + !response.headers.get('content-type')?.startsWith('text/event-stream') + ) { + throw await responseError(response) + } + let failure = 'Generation failed. Please try again.' + for await (const frame of readJsonEvents(response, observer.signal)) { + if (observer.signal.aborted) return + const event = generationEvent(frame) + onEvent(event) + if (observer.signal.aborted) return + if (event.type === 'error') failure = event.data.message + if (event.type === 'done') { + if (event.data.state === 'failed') throw new Error(failure) + return event.data.result + } + } + if (!observer.signal.aborted) { + throw new Error('Generation connection ended before completion. Please try again.') + } + } catch (cause) { + if (!observer.signal.aborted) throw cause + } + } + + return { + result: run(), + detach() { + observer.abort() + } + } +} diff --git a/apps/web/src/lib/generation/types.ts b/apps/web/src/lib/generation/types.ts index 9eeda16c..afec42a0 100644 --- a/apps/web/src/lib/generation/types.ts +++ b/apps/web/src/lib/generation/types.ts @@ -1,11 +1,6 @@ import type { ProgressPhase } from '$lib/components/progress' -export type GenerationTarget = { kind: 'field'; key: string } | { kind: 'body' } - -export type GenerationResult = - | { kind: 'creation'; markdown: string } - | { kind: 'field'; key: string; value: unknown } - | { kind: 'body'; value: string } +export type GenerationResult = { markdown: string } export type GenerationEvent = | { type: 'state'; data: { state: 'running' } } diff --git a/apps/web/src/lib/server/generation.test.ts b/apps/web/src/lib/server/generation.test.ts index 00bcfc94..e5d9f709 100644 --- a/apps/web/src/lib/server/generation.test.ts +++ b/apps/web/src/lib/server/generation.test.ts @@ -1,11 +1,8 @@ import { afterEach, beforeEach, expect, test, vi } from 'vitest' import { + DEFAULT_GENERATION_LIMITS, Piece, extractFullMarkdown, - generateBody, - generateFieldValue, - generatePieceMetadata, - type GenerationOptions, type PieceFrontmatter, type PieceFrontmatterSchema } from '@luzzle/core' @@ -13,21 +10,20 @@ import type { GenerationEvent } from '$lib/generation/types' import { POST } from '../../routes/api/admin/generate/+server' const fixture = vi.hoisted(() => { - const limits = { - maxFiles: 2, - maxFileBytes: 50, - maxTotalFileBytes: 75, - maxOutputBytes: 1000, - maxFileProcessingMs: 300000 - } const ai = { provider: 'google' as const, api_key: 'server-secret' } return { - limits, ai, config: { ai: ai as typeof ai | undefined, pieces: [{ type: 'books', fields: { title: 'title' } }] }, + provider: { + client: vi.fn(), + generate: vi.fn(), + upload: vi.fn(), + get: vi.fn(), + delete: vi.fn() + }, storage: { stat: vi.fn(), readFile: vi.fn(), @@ -47,12 +43,20 @@ vi.mock('./constants', () => ({ MAX_CONCURRENT_GENERATIONS: 1, HEARTBEAT_MS: 15000 })) -vi.mock('@luzzle/core', async (original) => ({ +// Mock only the provider transport: core selection, validation, parsing and merging stay real. +vi.mock('../../../../core/node_modules/@google/genai', async (original) => ({ ...(await original>()), - DEFAULT_GENERATION_LIMITS: fixture.limits, - generatePieceMetadata: vi.fn(), - generateFieldValue: vi.fn(), - generateBody: vi.fn() + GoogleGenAI: vi.fn(function (options) { + fixture.provider.client(options) + return { + models: { generateContent: fixture.provider.generate }, + files: { + upload: fixture.provider.upload, + get: fixture.provider.get, + delete: fixture.provider.delete + } + } + }) })) vi.mock('$lib/server/database', () => { throw new Error('Generation must not import the database') @@ -62,19 +66,27 @@ vi.mock('@luzzle/web.jobs', () => { }) let schema: PieceFrontmatterSchema +const limits = { ...DEFAULT_GENERATION_LIMITS } +const source = + '---\ntitle: Unsaved\nbody: Metadata body\ncover: unfinished\n---\n\nActual unsaved body\n' beforeEach(() => { vi.clearAllMocks() fixture.config.ai = fixture.ai - fixture.limits.maxOutputBytes = 1000 fixture.config.pieces = [{ type: 'books', fields: { title: 'title' } }] + Object.assign(DEFAULT_GENERATION_LIMITS, { + maxFiles: 2, + maxFileBytes: 50, + maxTotalFileBytes: 75, + maxOutputBytes: 1000 + }) schema = { title: 'books', type: 'object', properties: { title: { type: 'string', minLength: 2 }, body: { type: 'string' }, - cover: { type: 'string' } + cover: { type: 'string', format: 'asset' } }, required: ['title'], additionalProperties: false @@ -83,44 +95,42 @@ beforeEach(() => { expect(file).toBe('.luzzle/schemas/books.json') return JSON.stringify(schema) }) - fixture.storage.stat.mockImplementation(async (file: string) => { - if (file === '.' || file === 'books') return { type: 'directory' } - if (file === 'books/disk.books.md') return { type: 'file' } - throw new Error('Not in the archive') + fixture.storage.stat.mockRejectedValue(new Error('Generation must not probe archive files')) + fixture.provider.generate.mockResolvedValue(completion('# Generated body')) + fixture.provider.upload.mockResolvedValue({ name: 'files/uploaded' }) + fixture.provider.get.mockResolvedValue({ + state: 'ACTIVE', + uri: 'https://provider/uploaded', + mimeType: 'application/pdf' }) - vi.mocked(generatePieceMetadata).mockImplementation(async (_key, _schema, _prompt, options) => { - await options?.onProgress?.({ phase: 'generation', message: 'Generating metadata' }) - await options?.onProgress?.({ phase: 'validation', message: 'Validating metadata' }) - return { title: 'Model title', cover: 'https://invalid.example/asset.jpg' } - }) - vi.mocked(generateFieldValue).mockResolvedValue('Generated field') - vi.mocked(generateBody).mockResolvedValue('# Generated body') + fixture.provider.delete.mockResolvedValue({}) vi.spyOn(Piece.prototype, 'setField').mockRejectedValue(new Error('No asset downloads')) + vi.spyOn(Piece.prototype, 'write').mockRejectedValue(new Error('No saving')) vi.spyOn(console, 'error').mockImplementation(() => {}) }) afterEach(() => { - fixture.limits.maxOutputBytes = 1000 + Object.assign(DEFAULT_GENERATION_LIMITS, limits) for (const method of ['writeFile', 'createFile', 'makeDirectory', 'delete'] as const) { expect(fixture.storage[method]).not.toHaveBeenCalled() } + expect(fixture.storage.stat).not.toHaveBeenCalled() expect(Piece.prototype.setField).not.toHaveBeenCalled() + expect(Piece.prototype.write).not.toHaveBeenCalled() vi.restoreAllMocks() }) -function form(mode: 'create' | 'edit' = 'create') { +function completion(text: string) { + return { candidates: [{ content: { parts: [{ text }] }, finishReason: 'STOP' }] } +} + +function form(keys?: string[]) { const data = new FormData() - data.set('mode', mode) data.set('instructions', 'Find accurate details') - if (mode === 'create') { - data.set('name', 'Entered title') - data.set('type', 'books') - data.set('directory', 'books') - } else { - data.set('file', 'books/disk.books.md') - data.set('source', JSON.stringify('---\ntitle: Unsaved\n---\n\nActual unsaved body\n')) - data.set('target', 'body') - } + data.set('file', 'books/disk.books.md') + data.set('source', JSON.stringify(source)) + data.set('target', keys ? 'fields' : 'body') + if (keys) data.set('fields', JSON.stringify(keys)) return data } @@ -143,188 +153,218 @@ async function events(response: Response): Promise { }) } -function noProviderCalls() { - expect(generatePieceMetadata).not.toHaveBeenCalled() - expect(generateFieldValue).not.toHaveBeenCalled() - expect(generateBody).not.toHaveBeenCalled() -} - -test('creation returns validated metadata with the entered title and an empty body, without writes', async () => { - const data = form() - data.append('files', new File(['attachment'], 'source.txt')) - data.set('api_key', 'untrusted-browser-key') - data.set('schema', JSON.stringify({ type: 'object' })) - const response = await post(request(data)) - expect(response.headers.get('content-type')).toBe('text/event-stream') - const frames = await events(response) +function result(frames: GenerationEvent[]) { const done = frames.at(-1) expect(done).toMatchObject({ type: 'done', data: { state: 'completed' } }) if (done?.type !== 'done' || done.data.state !== 'completed') throw new Error('Missing result') - const result = done.data.result - if (result.kind !== 'creation') throw new Error('Wrong result') - expect(await extractFullMarkdown(result.markdown)).toMatchObject({ - frontmatter: { title: 'Entered title', cover: 'https://invalid.example/asset.jpg' }, - markdown: '' + return done.data.result.markdown +} + +function failed(frames: GenerationEvent[]) { + expect(frames.filter((frame) => frame.type === 'done')).toEqual([ + { type: 'done', data: { state: 'failed' } } + ]) + expect(frames.some((frame) => frame.type === 'error')).toBe(true) + expect(JSON.stringify(frames)).not.toContain('"markdown"') +} + +test.each([{ keys: ['title'] }, { keys: ['title', 'body'] }])( + 'metadata generation merges selected fields in one operation: $keys', + async ({ keys }) => { + const generated = Object.fromEntries(keys.map((key) => [key, `Generated ${key}`])) + fixture.provider.generate.mockResolvedValue(completion(JSON.stringify(generated))) + const data = form(keys) + data.append('files', new File(['attachment'], 'source.txt')) + data.set('api_key', 'untrusted-browser-key') + data.set('schema', JSON.stringify({ type: 'object' })) + const response = await post(request(data)) + expect(response.headers.get('content-type')).toBe('text/event-stream') + const frames = await events(response) + expect(await extractFullMarkdown(result(frames))).toMatchObject({ + frontmatter: { title: 'Unsaved', body: 'Metadata body', cover: 'unfinished', ...generated }, + markdown: 'Actual unsaved body' + }) + expect(fixture.provider.generate).toHaveBeenCalledTimes(1) + const call = fixture.provider.generate.mock.calls[0][0] + expect(call.config.responseJsonSchema.properties).toEqual( + Object.fromEntries(keys.map((key) => [key, schema.properties[key]])) + ) + expect(call.contents).toContain('Find accurate details') + expect(call.contents.join('\n')).toContain(JSON.stringify(source)) + expect(call.contents.join('\n')).toContain('attachment') + expect(fixture.provider.client).toHaveBeenCalledWith( + expect.objectContaining({ apiKey: 'server-secret' }) + ) + expect(JSON.stringify(frames)).not.toContain('server-secret') + const phases = frames.filter((frame) => frame.type === 'phase').at(-1) + expect(phases?.data.map((phase) => [phase.phase, phase.status])).toEqual([ + ['preparation', 'completed'], + ['generation', 'completed'], + ['validation', 'completed'] + ]) + expect(fixture.storage.readFile).toHaveBeenCalledExactlyOnceWith( + '.luzzle/schemas/books.json', + 'text' + ) + } +) + +test('nullable generated fields complete with null-free metadata without overwriting existing values or saving', async () => { + schema.properties.title = { type: ['string', 'null'], minLength: 2 } + schema.properties.body = { type: ['string', 'null'] } + schema.properties.cover = { type: ['string', 'null'], format: 'asset' } + const input = + '---\ntitle: Unsaved\nbody: null\nexisting: null\nnested:\n removed: null\n values: [null, false, 0, "", "null"]\n---\n\nKeep literal null in the body.' + fixture.provider.generate.mockResolvedValue(completion('{"title":null,"body":null,"cover":null}')) + const data = form(['title', 'body', 'cover']) + data.set('source', JSON.stringify(input)) + const frames = await events(await post(request(data))) + expect(await extractFullMarkdown(result(frames))).toEqual({ + frontmatter: { + title: 'Unsaved', + nested: { values: [false, 0, '', 'null'] } + }, + markdown: 'Keep literal null in the body.' }) - expect(generatePieceMetadata).toHaveBeenCalledWith( - 'server-secret', - schema, - expect.stringContaining('Entered title: "Entered title"'), - { - files: [Buffer.from('attachment')], - onProgress: expect.any(Function) - } - ) - expect(JSON.stringify(frames)).not.toContain('server-secret') - // No examples/defaults are needed: creation does not initialize a placeholder piece. - expect(schema.properties.title).not.toHaveProperty('examples') + expect(fixture.provider.generate).toHaveBeenCalledTimes(1) + for (const method of ['writeFile', 'createFile', 'makeDirectory', 'delete'] as const) { + expect(fixture.storage[method]).not.toHaveBeenCalled() + } + expect(Piece.prototype.write).not.toHaveBeenCalled() }) -test('body generation receives the raw unsaved source, never disk content', async () => { - const data = form('edit') - const frames = await events(await post(request(data))) - expect(generateBody).toHaveBeenCalledWith( - 'server-secret', - { source: JSON.parse(data.get('source') as string), instructions: data.get('instructions') }, - { files: [], onProgress: expect.any(Function) } - ) - expect(frames.at(-1)).toEqual({ - type: 'done', - data: { state: 'completed', result: { kind: 'body', value: '# Generated body' } } +test('body append preserves unsaved metadata and notes without validating untouched values', async () => { + const frames = await events(await post()) + expect(result(frames)).toBe(`${source}\n\n# Generated body`) + expect(await extractFullMarkdown(result(frames))).toMatchObject({ + frontmatter: { title: 'Unsaved', body: 'Metadata body', cover: 'unfinished' }, + markdown: 'Actual unsaved body\n\n\n# Generated body' }) - expect(fixture.storage.readFile).toHaveBeenCalledTimes(1) + const call = fixture.provider.generate.mock.calls[0][0] + expect(call.config).not.toHaveProperty('responseJsonSchema') + expect(call.contents.join('\n')).toContain(JSON.stringify(source)) + expect(fixture.storage.readFile).not.toHaveBeenCalled() +}) + +test.each([ + '', + '---\r\n# Keep this comment\r\ntitle: "Unsaved"\r\nbody: Metadata body\r\ncover: unfinished\r\n---\r\n\r\n Unsaved notes \r\n' +])('body append returns a whole document retaining exact editor source: %j', async (input) => { + const body = '\n# New notes\n\n trailing whitespace \r\n' + fixture.provider.generate.mockResolvedValue(completion(body)) + const data = form() + data.set('source', JSON.stringify(input)) + const frames = await events(await post(request(data))) + expect(result(frames)).toBe(input ? `${input}\n\n${body}` : body) }) test('a metadata field named body is not the Markdown body target', async () => { - const data = form('edit') - data.set('target', 'field') - data.set('key', 'body') - const frames = await events(await post(request(data))) - expect(generateBody).not.toHaveBeenCalled() - expect(generateFieldValue).toHaveBeenCalledWith( - 'server-secret', - { - key: 'body', - schema, - source: JSON.parse(data.get('source') as string), - instructions: data.get('instructions') - }, - { files: [], onProgress: expect.any(Function) } - ) - expect(frames.at(-1)).toMatchObject({ - type: 'done', - data: { result: { kind: 'field', key: 'body', value: 'Generated field' } } + fixture.provider.generate.mockResolvedValue(completion('{"body":"Generated metadata"}')) + const frames = await events(await post(request(form(['body'])))) + expect(await extractFullMarkdown(result(frames))).toMatchObject({ + frontmatter: { title: 'Unsaved', body: 'Generated metadata', cover: 'unfinished' }, + markdown: 'Actual unsaved body' }) }) -test.each([null, '', 'delete', new File(['create'], 'mode.txt')])( - 'rejects invalid mode %s before choosing a generator and releases capacity', - async (mode) => { +test.each(['http', 'https'])( + 'generated %s assets are accepted without downloading or saving them', + async (protocol) => { + const cover = `${protocol}://example.test/cover.jpg` + fixture.provider.generate.mockResolvedValue(completion(JSON.stringify({ cover }))) + const frames = await events(await post(request(form(['cover'])))) + expect(await extractFullMarkdown(result(frames))).toMatchObject({ + frontmatter: { title: 'Unsaved', cover } + }) + } +) + +test.each([ + '{"title":"Valid","cover":"cover.jpg"}', + '{"title":"X","cover":"https://example.test/cover.jpg"}', + '{"title":"Valid"}', + '{"title":"Valid","cover":42}' +])('validates every selected field and never returns partial metadata: %s', async (output) => { + fixture.provider.generate.mockResolvedValue(completion(output)) + failed(await events(await post(request(form(['title', 'cover']))))) +}) + +test('invalid editor YAML yields no usable merged fields document', async () => { + const data = form(['title']) + data.set('source', JSON.stringify('---\ntitle: [unfinished\n---\n\nUnsaved body\n')) + fixture.provider.generate.mockResolvedValue(completion('{"title":"Generated"}')) + failed(await events(await post(request(data)))) +}) + +test('body append preserves even unfinished editor YAML without parsing it', async () => { + const input = '---\ntitle: [unfinished\n---\n\nUnsaved body\n' + const data = form() + data.set('source', JSON.stringify(input)) + fixture.provider.generate.mockResolvedValue(completion('Generated body')) + expect(result(await events(await post(request(data))))).toBe(`${input}\n\nGenerated body`) +}) + +test.each(['unknown', 'toString', '__proto__', 'constructor', 'prototype', 'title.nested'])( + 'rejects field %s before provider work', + async (key) => { + await expect(post(request(form(['title', key])))).rejects.toMatchObject({ status: 400 }) + expect(fixture.provider.client).not.toHaveBeenCalled() + expect(fixture.provider.upload).not.toHaveBeenCalled() + expect(fixture.provider.generate).not.toHaveBeenCalled() + } +) + +test.each(['books/disk.unknown.md', '.assets/disk.books.md', 'file.books.txt', 'books'])( + 'rejects an invalid metadata schema identity %s', + async (file) => { + const data = form(['title']) + data.set('file', file) + await expect(post(request(data))).rejects.toMatchObject({ status: 400 }) + expect(fixture.provider.generate).not.toHaveBeenCalled() + } +) + +test.each([null, '', 'field', 'unknown', new File(['body'], 'target.txt')])( + 'invalid target %s releases capacity before provider work', + async (target) => { const data = form() - if (mode === null) data.delete('mode') - else data.set('mode', mode) + data.delete('target') + if (target !== null) data.set('target', target) await expect(post(request(data))).rejects.toMatchObject({ status: 400 }) expect(fixture.storage.readFile).not.toHaveBeenCalled() expect(fixture.storage.stat).not.toHaveBeenCalled() - noProviderCalls() - expect((await events(await post())).at(-1)).toMatchObject({ data: { state: 'completed' } }) + expect(fixture.provider.generate).not.toHaveBeenCalled() + result(await events(await post())) } ) -test.each(['unknown', 'toString', '__proto__', 'title.nested'])( - 'rejects field %s before provider work', - async (key) => { - const data = form('edit') - data.set('target', 'field') - data.set('key', key) - await expect(post(request(data))).rejects.toMatchObject({ status: 400 }) - noProviderCalls() - } -) - -test.each([ - 'missing.books.md', - '../outside.books.md', - 'books/disk.unknown.md', - 'assets/disk.books.md', - 'file.books.txt' -])('rejects invalid existing identity %s', async (file) => { - const data = form('edit') - data.set('file', file) - await expect(post(request(data))).rejects.toMatchObject({ status: 400 }) - noProviderCalls() -}) - -test.each([ - ['type', 'unknown'], - ['directory', '../outside'], - ['directory', 'books/disk.books.md'] -])('rejects bad creation %s', async (field, value) => { - const data = form() - data.set(field, value) - await expect(post(request(data))).rejects.toMatchObject({ status: 400 }) - noProviderCalls() -}) - -test('rejects a missing schema and releases capacity', async () => { +test('rejects a missing field schema and releases capacity', async () => { fixture.storage.readFile.mockRejectedValueOnce(new Error('missing schema')) - await expect(post()).rejects.toMatchObject({ status: 400 }) - noProviderCalls() - expect((await events(await post())).at(-1)).toMatchObject({ data: { state: 'completed' } }) + await expect(post(request(form(['title'])))).rejects.toMatchObject({ status: 400 }) + expect(fixture.provider.generate).not.toHaveBeenCalled() + result(await events(await post())) }) -test.each(['missing', '__proto__.title'])( - 'rejects unsupported title configuration %s', - async (title) => { - fixture.config.pieces[0].fields.title = title - await expect(post()).rejects.toMatchObject({ status: 400 }) - noProviderCalls() +test.each([0, -1])( + 'checks the appended document output cap at its boundary (%i bytes)', + async (offset) => { + const body = 'Small body' + const markdown = `${source}\n\n${body}` + const maxOutputBytes = Buffer.byteLength(JSON.stringify({ markdown })) + offset + Object.assign(DEFAULT_GENERATION_LIMITS, { maxOutputBytes }) + expect(Buffer.byteLength(JSON.stringify({ markdown: source }))).toBeLessThan(maxOutputBytes) + expect(Buffer.byteLength(body)).toBeLessThan(maxOutputBytes) + fixture.provider.generate.mockResolvedValue(completion(body)) + const frames = await events(await post()) + if (offset === 0) expect(result(frames)).toBe(markdown) + else failed(frames) } ) -test('supports a configured nested title without asset-setting operations', async () => { - schema = { - title: 'books', - type: 'object', - properties: { - details: { type: 'object', properties: { name: { type: 'string' } }, required: ['name'] } - }, - required: ['details'] - } - fixture.config.pieces[0].fields.title = 'details.name' - vi.mocked(generatePieceMetadata).mockResolvedValue({ details: { name: 'Model name' } }) +test('provider failures are terminal and do not expose provider details', async () => { + fixture.provider.generate.mockRejectedValueOnce(new Error('secret provider request details')) const frames = await events(await post()) - expect(JSON.stringify(frames.at(-1))).toContain('Entered title') - expect(JSON.stringify(frames.at(-1))).not.toContain('Model name') -}) - -test('revalidates the entered title against the completed metadata', async () => { - const data = form() - data.set('name', 'X') // Too short for the schema. - const frames = await events(await post(request(data))) - expect(frames.at(-1)).toEqual({ type: 'done', data: { state: 'failed' } }) - expect(frames.some((frame) => frame.type === 'error')).toBe(true) - expect(JSON.stringify(frames)).not.toContain('cover') -}) - -test('caps the final serialized result as well as the core completion', async () => { - fixture.limits.maxOutputBytes = 10 - const frames = await events(await post()) - expect(frames.at(-1)).toEqual({ type: 'done', data: { state: 'failed' } }) -}) - -test('invalid core completions are terminal failures, never partial results', async () => { - vi.mocked(generateFieldValue).mockImplementation(async (_key, _request, options) => { - await options?.onProgress?.({ phase: 'validation', message: 'Validating generated field' }) - throw new Error('Generated field is invalid; secret provider request details') - }) - const data = form('edit') - data.set('target', 'field') - data.set('key', 'title') - const frames = await events(await post(request(data))) - expect(frames.filter((event) => event.type === 'done')).toEqual([ - { type: 'done', data: { state: 'failed' } } - ]) + failed(frames) expect(JSON.stringify(frames)).not.toContain('secret provider request details') }) @@ -333,7 +373,7 @@ test('unconfigured AI rejects without reading the body', async () => { const req = request() await expect(post(req)).rejects.toMatchObject({ status: 503 }) expect(req.bodyUsed).toBe(false) - noProviderCalls() + expect(fixture.provider.generate).not.toHaveBeenCalled() }) test('reserves capacity before multipart parsing and releases it after malformed input', async () => { @@ -354,34 +394,133 @@ test('reserves capacity before multipart parsing and releases it after malformed expect(rejected.bodyUsed).toBe(false) stream.error(new Error('Incomplete upload')) await expect(pending).rejects.toThrow('Incomplete upload') - noProviderCalls() - expect((await events(await post())).at(-1)).toMatchObject({ data: { state: 'completed' } }) + expect(fixture.provider.generate).not.toHaveBeenCalled() + result(await events(await post())) }) test.each(['cancel', 'abort', 'detached failure'] as const)( 'keeps the slot through provider cleanup after %s', async (disconnect) => { - let settle!: (value: string) => void + let settle!: (value: ReturnType) => void let fail!: (cause: Error) => void - let received: GenerationOptions | undefined - vi.mocked(generateBody).mockImplementationOnce(async (_key, _context, options) => { - received = options - return new Promise((resolve, reject) => { - settle = resolve - fail = reject - }) - }) + let cleanup!: () => void + fixture.provider.generate.mockImplementationOnce( + () => + new Promise((resolve, reject) => { + settle = resolve + fail = reject + }) + ) + fixture.provider.delete.mockImplementationOnce( + () => + new Promise((resolve) => { + cleanup = resolve + }) + ) + const data = form() + data.append('files', new File(['%PDF-1.7\nreference'], 'reference.pdf')) const abort = new AbortController() - const response = await post(request(form('edit'), abort.signal)) + const response = await post(request(data, abort.signal)) + await vi.waitFor(() => expect(fixture.provider.generate).toHaveBeenCalledTimes(1)) if (disconnect === 'abort') abort.abort() else await response.body!.cancel() - await received?.onProgress?.({ phase: 'generation', message: 'Still working' }) await expect(post()).rejects.toMatchObject({ status: 429 }) - // Settling this fake core promise represents generation AND provider-file cleanup finishing. if (disconnect === 'detached failure') fail(new Error('provider failed after disconnect')) - else settle('Abandoned result') + else settle(completion('Abandoned result')) + await vi.waitFor(() => expect(fixture.provider.delete).toHaveBeenCalledTimes(1)) + await expect(post()).rejects.toMatchObject({ status: 429 }) + cleanup() await vi.waitFor(async () => { - expect((await events(await post())).at(-1)).toMatchObject({ data: { state: 'completed' } }) + result(await events(await post())) }) } ) + +test('metadata uses the configured schema without requiring a persisted piece', async () => { + const data = form(['title']) + data.set('file', 'not-created.books.md') + fixture.provider.generate.mockResolvedValue(completion('{"title":"Generated"}')) + const markdown = result(await events(await post(request(data)))) + expect((await extractFullMarkdown(markdown)).frontmatter.title).toBe('Generated') + expect(fixture.storage.readFile).toHaveBeenCalledExactlyOnceWith( + '.luzzle/schemas/books.json', + 'text' + ) +}) + +test('body generation needs neither a file identity nor an archive schema', async () => { + fixture.config.pieces = [] + fixture.storage.readFile.mockRejectedValue(new Error('Archive unavailable')) + const data = form() + data.delete('file') + expect(result(await events(await post(request(data))))).toBe(`${source}\n\n# Generated body`) + expect(fixture.storage.readFile).not.toHaveBeenCalled() +}) + +test.each([null, '', ' ', new File(['path'], 'identity.txt')])( + 'metadata requires a text file identity to select its schema: %s', + async (file) => { + const data = form(['title']) + data.delete('file') + if (file !== null) data.set('file', file) + await expect(post(request(data))).rejects.toMatchObject({ status: 400 }) + expect(fixture.storage.readFile).not.toHaveBeenCalled() + expect(fixture.provider.client).not.toHaveBeenCalled() + result(await events(await post())) + } +) + +test.each(['body', 'fields'] as const)( + '%s preflight checks input and attachments before any provider work', + async (target) => { + const data = form(target === 'fields' ? ['title'] : undefined) + data.append('files', new File([new Uint8Array(51)], 'oversized.bin')) + await expect(post(request(data))).rejects.toMatchObject({ status: 413 }) + expect(fixture.provider.client).not.toHaveBeenCalled() + expect(fixture.provider.upload).not.toHaveBeenCalled() + expect(fixture.provider.generate).not.toHaveBeenCalled() + result(await events(await post())) + } +) + +test('field selection validation is reported after the core promise settles', async () => { + fixture.provider.generate.mockResolvedValue(completion('{"title":"X"}')) + const frames = await events(await post(request(form(['title'])))) + failed(frames) + expect(frames).toContainEqual({ + type: 'error', + data: { + message: expect.stringMatching(/Generated field selection.*\/title.*must NOT have fewer/) + } + }) + expect(console.error).toHaveBeenCalledWith( + 'AI generation failed.', + expect.objectContaining({ + phase: 'validation', + error: 'GenerationValidationError', + validation: expect.stringContaining('/title') + }) + ) + result(await events(await post())) +}) + +test('logs an upload failure HTTP status from its cause without exposing provider details', async () => { + const providerError = Object.assign(new Error('private prompt and api-key=secret'), { + status: 403 + }) + fixture.provider.upload.mockRejectedValueOnce(providerError) + const data = form() + data.append('files', new File(['%PDF-1.7\nreference'], 'reference.pdf')) + const frames = await events(await post(request(data))) + failed(frames) + expect(console.error).toHaveBeenCalledWith( + 'AI generation failed.', + expect.objectContaining({ phase: 'preparation', status: 403 }) + ) + expect(JSON.stringify(frames)).not.toMatch(/private prompt|api-key=secret/) + expect(JSON.stringify(vi.mocked(console.error).mock.calls)).not.toMatch( + /private prompt|api-key=secret/ + ) + expect(fixture.provider.generate).not.toHaveBeenCalled() + result(await events(await post())) +}) diff --git a/apps/web/src/lib/server/generation.ts b/apps/web/src/lib/server/generation.ts index 7c3de0c2..6868f3cc 100644 --- a/apps/web/src/lib/server/generation.ts +++ b/apps/web/src/lib/server/generation.ts @@ -1,8 +1,23 @@ import { error } from '@sveltejs/kit' +import { + generatePieceBody, + generatePieceFrontmatter, + mergeGeneratedFields, + appendGeneratedBody, + type GenerationProgress, + type PieceFrontmatter, + type PieceFrontmatterSchema +} from '@luzzle/core' +import type { GenerationResult } from '$lib/generation/types' import { config } from './config' -import { readCreateInput, readEditInput } from './generation/input' -import { createDraftGenerator, createEditorGenerator, type Generate } from './generation/generator' -import { generationStream } from './generation/stream' +import { getPieces } from './pieces' +import { + readFieldsInput, + readBodyInput, + type FieldsGenerationInput, + type BodyGenerationInput +} from './generation/input' +import { createGenerationStream } from './generation/stream' import { MAX_CONCURRENT_GENERATIONS } from './constants' let active = 0 @@ -17,20 +32,95 @@ export async function generateResponse(request: Request): Promise { const release = () => active-- try { const form = await request.formData() - const mode = form.get('mode') - let generate: Generate - if (mode === 'create') { - const input = await readCreateInput(form) - generate = await createDraftGenerator(input, ai.api_key) - } else if (mode === 'edit') { - const input = await readEditInput(form) - generate = await createEditorGenerator(input, ai.api_key) - } else { - error(400, 'Choose creation or editing.') + const target = form.get('target') + if (target === 'fields') { + return await generateFieldsResponse(request, form, ai.api_key, release) } - return generationStream(request, generate, release) + if (target === 'body') { + return await generateBodyResponse(request, form, ai.api_key, release) + } + error(400, 'Choose metadata fields or the body.') } catch (cause) { release() throw cause } } + +async function generateFieldsResponse( + request: Request, + form: FormData, + apiKey: string, + release: () => void +): Promise { + const file = form.get('file') + if (typeof file !== 'string' || !file.trim()) error(400, 'Piece file is required.') + const input = await readFieldsInput(form) + const schema = await loadFieldSchema(file) + const unknownKeys = input.keys.some( + (key) => key === '__proto__' || !Object.hasOwn(schema.properties, key) + ) + if (unknownKeys) error(400, 'Choose top-level metadata fields.') + + const stream = createGenerationStream(request) + const work = generateFieldsDocument(apiKey, input, schema, stream.progress) + void stream.sendResult(work, release) + return stream.response +} + +async function generateBodyResponse( + request: Request, + form: FormData, + apiKey: string, + release: () => void +): Promise { + const input = await readBodyInput(form) + const stream = createGenerationStream(request) + const work = generateBodyDocument(apiKey, input, stream.progress) + void stream.sendResult(work, release) + return stream.response +} + +async function loadFieldSchema(file: string): Promise> { + const pieces = getPieces() + const filename = pieces.parseFilename(file) + if (filename.format !== '.md' || pieces.isAsset(file)) error(400, 'Choose an archive piece.') + const type = filename.type + if (!type || !config.pieces.some((entry) => entry.type === type)) { + error(400, 'Choose a configured piece type.') + } + try { + const piece = await pieces.getPiece(type) + return piece.schema + } catch { + error(400, 'Could not load the piece schema.') + } +} + +async function generateFieldsDocument( + apiKey: string, + input: FieldsGenerationInput, + schema: PieceFrontmatterSchema, + progress: (event: GenerationProgress) => void +): Promise { + const fields = await generatePieceFrontmatter( + apiKey, + { source: input.source, instructions: input.instructions, schema, keys: input.keys }, + { files: input.files, onProgress: progress } + ) + const markdown = await mergeGeneratedFields(input.source, fields) + return { markdown } +} + +async function generateBodyDocument( + apiKey: string, + input: BodyGenerationInput, + progress: (event: GenerationProgress) => void +): Promise { + const body = await generatePieceBody( + apiKey, + { source: input.source, instructions: input.instructions }, + { files: input.files, onProgress: progress } + ) + const markdown = appendGeneratedBody(input.source, body) + return { markdown } +} diff --git a/apps/web/src/lib/server/generation/generator.ts b/apps/web/src/lib/server/generation/generator.ts deleted file mode 100644 index f70a854c..00000000 --- a/apps/web/src/lib/server/generation/generator.ts +++ /dev/null @@ -1,103 +0,0 @@ -import { - findFrontmatterField, - generateBody, - generateFieldValue, - generatePieceMetadata, - makePieceMarkdown, - makePieceMarkdownString, - setFrontmatterValue, - type GenerationProgress -} from '@luzzle/core' -import { error } from '@sveltejs/kit' -import type { GenerationResult } from '$lib/generation/types' -import { config } from '../config' -import { getPieces } from '../pieces' -import { getStorage } from '../storage' -import type { CreateGenerationInput, EditGenerationInput } from './input' - -export type Generate = (progress: (event: GenerationProgress) => void) => Promise - -export async function createDraftGenerator( - input: CreateGenerationInput, - apiKey: string -): Promise { - const piece = await getConfiguredPiece(input.type) - await archiveEntry(input.directory || '.', 'directory') - const title = config.pieces.find((entry) => entry.type === input.type)?.fields.title - if ( - !title || - title.split('.').some((part) => ['__proto__', 'constructor', 'prototype'].includes(part)) || - findFrontmatterField(piece.fields, title)?.type !== 'string' - ) { - error(400, 'This piece type needs a string title field.') - } - return async (progress) => { - const frontmatter = await generatePieceMetadata( - apiKey, - piece.schema, - `${input.instructions}\n\nEntered title: ${JSON.stringify(input.name)}`, - { files: input.files, onProgress: progress } - ) - // Do not use setField(): generation must not download assets or persist anything. - setFrontmatterValue(frontmatter, title, input.name) - // The destination is derived afresh on explicit Create, not reserved during generation. - const draft = makePieceMarkdown('', input.type, '', frontmatter) - if (!piece.validate(draft).isValid) throw new Error('Invalid generated draft.') - return { kind: 'creation', markdown: makePieceMarkdownString(draft) } - } -} - -export async function createEditorGenerator( - input: EditGenerationInput, - apiKey: string -): Promise { - const pieces = getPieces() - const file = pieces.parseFilename(input.file) - if (file.format !== '.md' || pieces.isAsset(input.file)) { - error(400, 'Choose an archive piece.') - } - const piece = await getConfiguredPiece(file.type) - await archiveEntry(input.file, 'file') - const target = input.target - if ( - target.kind === 'field' && - (target.key === '__proto__' || !Object.hasOwn(piece.schema.properties, target.key)) - ) { - error(400, 'Choose a top-level metadata field.') - } - return async (progress) => { - const context = { source: input.source, instructions: input.instructions } - const generationOptions = { files: input.files, onProgress: progress } - if (target.kind === 'body') { - return { kind: 'body', value: await generateBody(apiKey, context, generationOptions) } - } - return { - kind: 'field', - key: target.key, - value: await generateFieldValue( - apiKey, - { ...context, schema: piece.schema, key: target.key }, - generationOptions - ) - } - } -} - -async function archiveEntry(path: string, type: 'file' | 'directory') { - try { - if ((await getStorage().stat(path)).type !== type) throw new Error('Wrong entry type') - } catch { - error(400, `Choose an existing archive ${type}.`) - } -} - -async function getConfiguredPiece(type: string | null) { - if (!type || !config.pieces.some((entry) => entry.type === type)) { - error(400, 'Choose a configured piece type.') - } - try { - return await getPieces().getPiece(type) - } catch { - error(400, 'Could not prepare the piece schema.') - } -} diff --git a/apps/web/src/lib/server/generation/input.test.ts b/apps/web/src/lib/server/generation/input.test.ts index d61ce1a3..5c029f22 100644 --- a/apps/web/src/lib/server/generation/input.test.ts +++ b/apps/web/src/lib/server/generation/input.test.ts @@ -1,5 +1,5 @@ import { afterEach, describe, expect, it, vi } from 'vitest' -import { readCreateInput, readEditInput } from './input' +import { readFieldsInput, readBodyInput } from './input' import { MAX_SOURCE_CHARS, MAX_INSTRUCTION_CHARS } from '../constants' vi.mock('@luzzle/core', () => ({ @@ -7,18 +7,10 @@ vi.mock('@luzzle/core', () => ({ })) vi.mock('../constants', () => ({ MAX_SOURCE_CHARS: 128, MAX_INSTRUCTION_CHARS: 32 })) -function createForm(): FormData { +function editorForm(): FormData { const form = new FormData() - form.set('name', 'Dune') - form.set('type', 'book') - return form -} - -function editForm(): FormData { - const form = new FormData() - form.set('file', 'library/dune.book.md') form.set('source', JSON.stringify('')) - form.set('target', 'body') + form.set('fields', JSON.stringify(['title'])) return form } @@ -28,167 +20,139 @@ function attach(form: FormData, size: number, name = 'reference.bin') { afterEach(() => vi.restoreAllMocks()) -describe('readCreateInput', () => { - it('accepts create input with optional fields omitted', async () => { - await expect(readCreateInput(createForm())).resolves.toEqual({ - name: 'Dune', - type: 'book', - directory: '', - instructions: '', - files: [] - }) +describe.each([ + ['fields', readFieldsInput], + ['body', readBodyInput] +] as const)('read %s input', (kind, readInput) => { + it.each(['', '---\ntitle: Dune\n---\n\n body\n', ' body\r\nsecond line\r\n'])( + 'preserves JSON-encoded editor source exactly: %j', + async (source) => { + const form = editorForm() + form.set('source', JSON.stringify(source)) + await expect(readInput(form)).resolves.toEqual({ + source, + ...(kind === 'fields' ? { keys: ['title'] } : {}), + instructions: '', + files: [] + }) + } + ) + + it('requires source text', async () => { + const form = editorForm() + form.delete('source') + await expect(readInput(form)).rejects.toMatchObject({ status: 400 }) + form.set('source', new File(['value'], 'value.txt')) + await expect(readInput(form)).rejects.toMatchObject({ status: 400 }) }) - it('preserves create text, returns binary Buffers and ignores unrelated fields', async () => { - const form = createForm() - form.set('directory', 'library/science fiction') - form.set('instructions', ' Keep the original tone. ') - form.set('unrelated', 'ignored') + it.each(['not json', 'null', '{}', '123'])( + 'rejects nonstring source JSON: %s', + async (source) => { + const form = editorForm() + form.set('source', source) + await expect(readInput(form)).rejects.toMatchObject({ status: 400 }) + } + ) + + it('bounds decoded source characters, not UTF-8 bytes', async () => { + const form = editorForm() + const source = 'é'.repeat(MAX_SOURCE_CHARS) + form.set('source', JSON.stringify(source)) + await expect(readInput(form)).resolves.toMatchObject({ source }) + form.set('source', JSON.stringify(source + 'é')) + await expect(readInput(form)).rejects.toMatchObject({ status: 413 }) + }) + + it('requires optional instructions to be text', async () => { + const form = editorForm() + form.set('instructions', new File(['value'], 'value.txt')) + await expect(readInput(form)).rejects.toMatchObject({ status: 400 }) + }) + + it('preserves whitespace and bounds instruction characters, not UTF-8 bytes', async () => { + const form = editorForm() + const instructions = ' ' + 'é'.repeat(MAX_INSTRUCTION_CHARS - 2) + ' ' + form.set('instructions', instructions) + await expect(readInput(form)).resolves.toMatchObject({ instructions }) + form.set('instructions', instructions + 'é') + await expect(readInput(form)).rejects.toMatchObject({ status: 413 }) + }) + + it('reads binary attachments as Buffers', async () => { + const form = editorForm() attach(form, 8) attach(form, 4) - const result = await readCreateInput(form) - expect(result).toEqual({ - name: 'Dune', - type: 'book', - directory: 'library/science fiction', - instructions: ' Keep the original tone. ', - files: [Buffer.alloc(8, 255), Buffer.alloc(4, 255)] - }) + const result = await readInput(form) + expect(result.files).toEqual([Buffer.alloc(8, 255), Buffer.alloc(4, 255)]) expect(result.files.every(Buffer.isBuffer)).toBe(true) }) - it.each(['name', 'type'])('requires nonblank text for %s', async (field) => { - const form = createForm() - form.delete(field) - await expect(readCreateInput(form)).rejects.toMatchObject({ status: 400 }) - for (const value of [' \t ', new File(['value'], 'value.txt')]) { - form.set(field, value) - await expect(readCreateInput(form)).rejects.toMatchObject({ status: 400 }) - } - }) - - it('requires optional directory to be text', async () => { - const form = createForm() - form.set('directory', new File(['value'], 'value.txt')) - await expect(readCreateInput(form)).rejects.toMatchObject({ status: 400 }) - }) - it.each([ ['attachment type', [1], 400], ['per-file bytes', [1, 9], 413], ['combined bytes', [8, 5], 413], ['file count', [1, 1, 1], 413] ] as const)('checks %s before reading any attachment buffers', async (reason, sizes, status) => { - const form = createForm() + const form = editorForm() for (const size of sizes) attach(form, size) const reads = form.getAll('files').map((file) => vi.spyOn(file as File, 'arrayBuffer')) if (reason === 'attachment type') form.append('files', 'not a file') - await expect(readCreateInput(form)).rejects.toMatchObject({ status }) + await expect(readInput(form)).rejects.toMatchObject({ status }) for (const bufferRead of reads) expect(bufferRead).not.toHaveBeenCalled() }) it('ignores empty files without counting them toward attachment limits', async () => { - const form = createForm() + const form = editorForm() for (let i = 0; i < 3; i++) attach(form, 0, 'empty.txt') attach(form, 0, '') attach(form, 8) attach(form, 4) - await expect(readCreateInput(form)).resolves.toMatchObject({ + await expect(readInput(form)).resolves.toMatchObject({ files: [Buffer.alloc(8, 255), Buffer.alloc(4, 255)] }) }) }) -describe('readEditInput', () => { - it.each(['', '---\ntitle: Dune\n---\n\n body\n', ' body\r\nsecond line\r\n'])( - 'preserves JSON-encoded editor source exactly: %j', - async (source) => { - const form = editForm() - form.set('source', JSON.stringify(source)) - await expect(readEditInput(form)).resolves.toEqual({ - file: 'library/dune.book.md', - source, - target: { kind: 'body' }, - instructions: '', - files: [] +describe('readFieldsInput', () => { + it.each([{ keys: ['title'] }, { keys: ['body'] }, { keys: ['title', 'body'] }])( + 'accepts one or more metadata fields, distinct from the body: $keys', + async ({ keys }) => { + const form = editorForm() + form.set('target', 'fields') + form.set('fields', JSON.stringify(keys)) + form.set('unrelated', 'ignored') + await expect(readFieldsInput(form)).resolves.toMatchObject({ + keys }) } ) - it.each(['title', 'body'])('keeps field target %s distinct from the body target', async (key) => { - const form = editForm() - form.set('target', 'field') - form.set('key', key) - form.set('unrelated', 'ignored') - await expect(readEditInput(form)).resolves.toMatchObject({ target: { kind: 'field', key } }) - }) - - it.each(['file', 'source', 'target'])('requires text for %s', async (field) => { - const form = editForm() - form.delete(field) - await expect(readEditInput(form)).rejects.toMatchObject({ status: 400 }) - form.set(field, new File(['value'], 'value.txt')) - await expect(readEditInput(form)).rejects.toMatchObject({ status: 400 }) - }) - - it('rejects a blank file', async () => { - const form = editForm() - form.set('file', ' \t ') - await expect(readEditInput(form)).rejects.toMatchObject({ status: 400 }) - }) - - it('rejects unsupported targets and requires a text key for field edits', async () => { - const form = editForm() - form.set('target', 'unknown') - await expect(readEditInput(form)).rejects.toMatchObject({ status: 400 }) - form.set('target', 'field') - await expect(readEditInput(form)).rejects.toMatchObject({ status: 400 }) - for (const key of ['', new File(['title'], 'key.txt')]) { - form.set('key', key) - await expect(readEditInput(form)).rejects.toMatchObject({ status: 400 }) - } - }) - - it.each(['not json', 'null', '{}', '123'])( - 'rejects nonstring source JSON: %s', - async (source) => { - const form = editForm() - form.set('source', source) - await expect(readEditInput(form)).rejects.toMatchObject({ status: 400 }) - } - ) - - it('bounds decoded source characters, not UTF-8 bytes', async () => { - const form = editForm() - const source = 'é'.repeat(MAX_SOURCE_CHARS) - form.set('source', JSON.stringify(source)) - await expect(readEditInput(form)).resolves.toMatchObject({ source }) - form.set('source', JSON.stringify(source + 'é')) - await expect(readEditInput(form)).rejects.toMatchObject({ status: 413 }) + it.each([ + null, + 'not json', + 'null', + '{}', + '"title"', + '[]', + '[""]', + '[" "]', + '["title", 1]', + '["title", "title"]', + new File(['["title"]'], 'fields.txt') + ])('requires a nonempty JSON array of distinct text fields: %s', async (fields) => { + const form = editorForm() + form.set('target', 'fields') + form.delete('fields') + if (fields !== null) form.set('fields', fields) + await expect(readFieldsInput(form)).rejects.toMatchObject({ status: 400 }) }) }) -describe('instructions in both readers', () => { - it('requires optional instructions to be text', async () => { - const create = createForm() - const edit = editForm() - create.set('instructions', new File(['value'], 'value.txt')) - edit.set('instructions', new File(['value'], 'value.txt')) - await expect(readCreateInput(create)).rejects.toMatchObject({ status: 400 }) - await expect(readEditInput(edit)).rejects.toMatchObject({ status: 400 }) - }) - - it('preserves whitespace and bounds characters, not UTF-8 bytes', async () => { - const create = createForm() - const edit = editForm() - const instructions = ' ' + 'é'.repeat(MAX_INSTRUCTION_CHARS - 2) + ' ' - create.set('instructions', instructions) - edit.set('instructions', instructions) - await expect(readCreateInput(create)).resolves.toMatchObject({ instructions }) - await expect(readEditInput(edit)).resolves.toMatchObject({ instructions }) - create.set('instructions', instructions + 'é') - edit.set('instructions', instructions + 'é') - await expect(readCreateInput(create)).rejects.toMatchObject({ status: 413 }) - await expect(readEditInput(edit)).rejects.toMatchObject({ status: 413 }) - }) +it('body input has no field selection, even when fields are submitted', async () => { + const form = editorForm() + form.set('fields', 'not a selection') + const input = await readBodyInput(form) + expect(input).not.toHaveProperty('keys') + expect(input).not.toHaveProperty('target') }) diff --git a/apps/web/src/lib/server/generation/input.ts b/apps/web/src/lib/server/generation/input.ts index 975728c9..7dc458d7 100644 --- a/apps/web/src/lib/server/generation/input.ts +++ b/apps/web/src/lib/server/generation/input.ts @@ -1,58 +1,62 @@ import { error } from '@sveltejs/kit' import { DEFAULT_GENERATION_LIMITS } from '@luzzle/core' -import type { GenerationTarget } from '$lib/generation/types' import { MAX_SOURCE_CHARS, MAX_INSTRUCTION_CHARS } from '../constants' -export type CreateGenerationInput = { - name: string - type: string - directory: string +export type FieldsGenerationInput = { + source: string instructions: string files: Buffer[] + keys: string[] } -export type EditGenerationInput = { - file: string +export type BodyGenerationInput = { source: string - target: GenerationTarget instructions: string files: Buffer[] } -export async function readCreateInput(form: FormData): Promise { - const name = form.get('name') - const type = form.get('type') - const directory = form.get('directory') ?? '' - const instructions = form.get('instructions') ?? '' - if (typeof name !== 'string' || !name.trim()) error(400, 'Name is required.') - if (typeof type !== 'string' || !type.trim()) error(400, 'Piece type is required.') - if (typeof directory !== 'string') error(400, 'Directory must be text.') - if (typeof instructions !== 'string') error(400, 'Instructions must be text.') - if (instructions.length > MAX_INSTRUCTION_CHARS) error(413, 'Instructions are too long.') - return { name, type, directory, instructions, files: await readAttachments(form) } +export async function readFieldsInput(form: FormData): Promise { + const instructions = readInstructions(form) + const source = readSource(form) + + const encodedFields = form.get('fields') + if (typeof encodedFields !== 'string') error(400, 'Choose metadata fields.') + let keys: unknown + try { + keys = JSON.parse(encodedFields) + } catch { + error(400, 'Invalid metadata fields.') + } + if ( + !Array.isArray(keys) || + keys.length === 0 || + !keys.every((key): key is string => typeof key === 'string' && !!key.trim()) || + new Set(keys).size !== keys.length + ) { + error(400, 'Choose distinct metadata fields.') + } + const files = await readAttachments(form) + return { source, instructions, keys, files } } -export async function readEditInput(form: FormData): Promise { - const file = form.get('file') - const encodedSource = form.get('source') - const targetKind = form.get('target') - const key = form.get('key') +export async function readBodyInput(form: FormData): Promise { + const instructions = readInstructions(form) + const source = readSource(form) + const files = await readAttachments(form) + return { source, instructions, files } +} + +function readInstructions(form: FormData): string { const instructions = form.get('instructions') ?? '' - if (typeof file !== 'string' || !file.trim()) error(400, 'Piece file is required.') - if (typeof encodedSource !== 'string') error(400, 'Editor source is required.') if (typeof instructions !== 'string') error(400, 'Instructions must be text.') if (instructions.length > MAX_INSTRUCTION_CHARS) error(413, 'Instructions are too long.') + return instructions +} - let target: GenerationTarget - if (targetKind === 'body') { - target = { kind: 'body' } - } else if (targetKind === 'field' && typeof key === 'string' && key) { - target = { kind: 'field', key } - } else { - error(400, 'Choose a field or the body.') - } - +function readSource(form: FormData): string { // JSON preserves the exact editor buffer; multipart text normalizes line endings. + const encodedSource = form.get('source') + if (typeof encodedSource !== 'string') error(400, 'Editor source is required.') let source: unknown try { source = JSON.parse(encodedSource) @@ -61,7 +65,7 @@ export async function readEditInput(form: FormData): Promise MAX_SOURCE_CHARS) error(413, 'Editor source is too long.') - return { file, source, target, instructions, files: await readAttachments(form) } + return source } async function readAttachments(form: FormData): Promise { diff --git a/apps/web/src/lib/server/generation/stream.test.ts b/apps/web/src/lib/server/generation/stream.test.ts index f872a5f5..6e80ac42 100644 --- a/apps/web/src/lib/server/generation/stream.test.ts +++ b/apps/web/src/lib/server/generation/stream.test.ts @@ -1,9 +1,9 @@ import { afterEach, beforeEach, describe, expect, test, vi } from 'vitest' +import { DEFAULT_GENERATION_LIMITS, GenerationValidationError } from '@luzzle/core' import type { GenerationEvent, GenerationResult } from '$lib/generation/types' -import type { Generate } from './generator' -import { generationStream } from './stream' +import { createGenerationStream } from './stream' -const result: GenerationResult = { kind: 'body', value: 'café\n🧩 日本語' } +const result: GenerationResult = { markdown: 'café\n🧩 日本語' } const decoder = new TextDecoder() function deferred() { @@ -42,18 +42,13 @@ async function drain(reader: ReadableStreamDefaultReader) { } } -function setup(generate?: Generate, abort = new AbortController()) { +function setup(abort = new AbortController()) { const completion = deferred() - let progress!: Parameters[0] - const fake = vi.fn((notify) => { - progress = notify - return generate ? generate(notify) : completion.promise - }) const request = new Request('http://localhost/api/admin/generate', { signal: abort.signal }) const removeListener = vi.spyOn(request.signal, 'removeEventListener') const release = vi.fn() - const response = generationStream(request, fake, release) - return { completion, progress, fake, request, abort, removeListener, release, response } + const stream = createGenerationStream(request) + return { completion, request, abort, removeListener, release, ...stream } } beforeEach(() => { @@ -67,9 +62,10 @@ afterEach(() => { vi.restoreAllMocks() }) -describe('generationStream', () => { +describe('createGenerationStream', () => { test('streams early state and progress, then exactly one validated UTF-8 result', async () => { const s = setup() + void s.sendResult(s.completion.promise, s.release) const reader = s.response.body!.getReader() expect(s.response).toBeInstanceOf(Response) expect(Object.fromEntries(s.response.headers)).toEqual({ @@ -176,7 +172,8 @@ describe('generationStream', () => { }) test('supports native Response.text() with multibyte output', async () => { - const s = setup(async () => result) + const s = setup() + void s.sendResult(Promise.resolve(result), s.release) const text = await s.response.text() expect(text).toContain('café\\n🧩 日本語') expect(events(text).at(-1)).toEqual({ @@ -186,18 +183,52 @@ describe('generationStream', () => { expect(s.release).toHaveBeenCalledTimes(1) }) + test.each([0, -1])( + 'caps the serialized UTF-8 result at the byte boundary (%i)', + async (offset) => { + const originalLimit = DEFAULT_GENERATION_LIMITS.maxOutputBytes + Object.assign(DEFAULT_GENERATION_LIMITS, { + maxOutputBytes: Buffer.byteLength(JSON.stringify(result)) + offset + }) + vi.spyOn(console, 'error').mockImplementation(() => {}) + try { + const s = setup() + void s.sendResult(Promise.resolve(result), s.release) + const frames = events(await s.response.text()) + expect(frames.filter((frame) => frame.type === 'done')).toEqual([ + { + type: 'done', + data: offset === 0 ? { state: 'completed', result } : { state: 'failed' } + } + ]) + if (offset < 0) { + expect(frames).toContainEqual({ + type: 'error', + data: { message: 'Generation failed. Please try again.' } + }) + expect(JSON.stringify(frames)).not.toContain('markdown') + } + expect(s.release).toHaveBeenCalledOnce() + expect(vi.getTimerCount()).toBe(0) + } finally { + Object.assign(DEFAULT_GENERATION_LIMITS, { maxOutputBytes: originalLimit }) + } + } + ) + test.each(['reject', 'throw'] as const)( '%s sends one failed done, a safe error and no result', async (mode) => { const secret = 'api-key=secret prompt=private filename=/private/scan.pdf provider-stack' const log = vi.spyOn(console, 'error').mockImplementation(() => {}) - const s = setup( + const s = setup() + const work = mode === 'throw' - ? () => { + ? (async () => { throw new Error(secret) - } - : undefined - ) + })() + : s.completion.promise + void s.sendResult(work, s.release) if (mode === 'reject') { s.progress({ phase: 'validation', message: 'Checking output' }) vi.setSystemTime(1_200) @@ -217,14 +248,72 @@ describe('generationStream', () => { finished_at: mode === 'reject' ? 1_200 : 1_000 }) expect(text).not.toMatch(/result|api-key|private|provider-stack/) - expect(log.mock.calls).toEqual([['AI generation failed.']]) + expect(log).toHaveBeenCalledWith( + 'AI generation failed.', + expect.objectContaining({ + phase: mode === 'reject' ? 'validation' : 'preparation', + error: 'Error', + stack: expect.stringContaining('stream.test.ts') + }) + ) + expect(JSON.stringify(log.mock.calls)).not.toMatch(/api-key|prompt=private|provider-stack/) expect(s.release).toHaveBeenCalledTimes(1) expect(vi.getTimerCount()).toBe(0) } ) + test('reports the field and rule for a local schema failure without returning a result', async () => { + const log = vi.spyOn(console, 'error').mockImplementation(() => {}) + const message = 'Generated frontmatter does not match schema: /title must be string' + const s = setup() + const work = (async () => { + s.progress({ phase: 'validation', message: 'Validating metadata' }) + throw new GenerationValidationError(message) + })() + void s.sendResult(work, s.release) + const frames = events(await s.response.text()) + expect(frames).toContainEqual({ type: 'error', data: { message } }) + expect(frames.at(-1)).toEqual({ type: 'done', data: { state: 'failed' } }) + expect(log).toHaveBeenCalledWith( + 'AI generation failed.', + expect.objectContaining({ + phase: 'validation', + validation: message, + error: 'GenerationValidationError' + }) + ) + expect(s.release).toHaveBeenCalledOnce() + }) + + test.each([false, true])('logs provider status safely (wrapped cause: %s)', async (wrapped) => { + const log = vi.spyOn(console, 'error').mockImplementation(() => {}) + const s = setup() + const work = (async () => { + s.progress({ phase: 'generation', message: 'Generating content' }) + const providerError = Object.assign(new Error('private request\nsecond private line'), { + status: 404, + request: { apiKey: 'secret-key', contents: 'private prompt' } + }) + if (wrapped) throw new Error('Could not upload attachment.', { cause: providerError }) + throw providerError + })() + void s.sendResult(work, s.release) + await s.response.text() + expect(log).toHaveBeenCalledWith( + 'AI generation failed.', + expect.objectContaining({ + phase: 'generation', + error: 'Error', + status: 404, + stack: expect.stringContaining('stream.test.ts') + }) + ) + expect(JSON.stringify(log.mock.calls)).not.toMatch(/private|secret-key/) + }) + test('heartbeats every 15 seconds through and beyond 300 seconds, not an operation deadline', async () => { const s = setup() + void s.sendResult(s.completion.promise, s.release) const reader = s.response.body!.getReader() await read(reader) await read(reader) @@ -250,6 +339,7 @@ describe('generationStream', () => { test('does not accumulate queued heartbeats behind an unread response', async () => { const s = setup() + void s.sendResult(s.completion.promise, s.release) await vi.advanceTimersByTimeAsync(300_000) const reader = s.response.body!.getReader() expect(events(await read(reader))[0].type).toBe('state') @@ -286,16 +376,18 @@ describe('generationStream', () => { const unhandled = vi.fn() process.on('unhandledRejection', unhandled) try { - const s = setup(async (progress) => { + const s = setup() + const work = (async () => { try { const value = await provider.promise - progress({ phase: 'validation', message: 'Checking abandoned output' }) + s.progress({ phase: 'validation', message: 'Checking abandoned output' }) return value } finally { cleaning() await cleanup.promise } - }) + })() + void s.sendResult(work, s.release) const reader = s.response.body!.getReader() await read(reader) await read(reader) @@ -306,7 +398,6 @@ describe('generationStream', () => { expect(s.removeListener).toHaveBeenCalledWith('abort', expect.any(Function)) expect(vi.getTimerCount()).toBe(0) await vi.advanceTimersByTimeAsync(315_000) - expect(s.fake).toHaveBeenCalledTimes(1) expect(cleaning).not.toHaveBeenCalled() expect(s.release).not.toHaveBeenCalled() @@ -320,7 +411,18 @@ describe('generationStream', () => { await vi.advanceTimersByTimeAsync(0) expect(s.release).toHaveBeenCalledTimes(1) expect(await reader.read()).toEqual({ done: true, value: undefined }) - expect(log.mock.calls).toEqual(outcome === 'success' ? [] : [['AI generation failed.']]) + if (outcome === 'success') { + expect(log).not.toHaveBeenCalled() + } else { + expect(log).toHaveBeenCalledWith( + 'AI generation failed.', + expect.objectContaining({ + phase: outcome === 'provider failure' ? 'preparation' : 'validation', + error: 'Error' + }) + ) + expect(JSON.stringify(log.mock.calls)).not.toMatch(/secret provider|secret cleanup/) + } expect(unhandled).not.toHaveBeenCalled() expect(vi.getTimerCount()).toBe(0) } finally { @@ -332,8 +434,14 @@ describe('generationStream', () => { test('an already-aborted accepted request still runs and releases only after settling', async () => { const abort = new AbortController() abort.abort() - const s = setup(undefined, abort) - expect(s.fake).toHaveBeenCalledTimes(1) + const s = setup(abort) + let started = false + const work = (async () => { + started = true + return await s.completion.promise + })() + void s.sendResult(work, s.release) + expect(started).toBe(true) expect(await s.response.body!.getReader().read()).toEqual({ done: true, value: undefined }) expect(s.removeListener).toHaveBeenCalledWith('abort', expect.any(Function)) expect(vi.getTimerCount()).toBe(0) @@ -353,6 +461,7 @@ describe('generationStream', () => { }) if (when === 'initial') fail() const s = setup() + void s.sendResult(s.completion.promise, s.release) const reader = s.response.body!.getReader() if (when === 'heartbeat') { await read(reader) diff --git a/apps/web/src/lib/server/generation/stream.ts b/apps/web/src/lib/server/generation/stream.ts index ec38521c..dc3910d2 100644 --- a/apps/web/src/lib/server/generation/stream.ts +++ b/apps/web/src/lib/server/generation/stream.ts @@ -1,16 +1,16 @@ -import { DEFAULT_GENERATION_LIMITS, type GenerationProgress } from '@luzzle/core' +import { + DEFAULT_GENERATION_LIMITS, + GenerationValidationError, + type GenerationProgress +} from '@luzzle/core' import type { ProgressPhase } from '$lib/components/progress' -import type { GenerationEvent } from '$lib/generation/types' +import type { GenerationEvent, GenerationResult } from '$lib/generation/types' import { createEventStream } from '../sse' -import type { Generate } from './generator' -export function generationStream( - request: Request, - generate: Generate, - release: () => void -): Response { +export function createGenerationStream(request: Request) { const events = createEventStream(request) const phases: ProgressPhase[] = [] + let currentPhase: GenerationProgress['phase'] = 'preparation' function emit(event: GenerationEvent) { events.emit(event.type, event.data) @@ -25,6 +25,7 @@ export function generationStream( } function progress(event: GenerationProgress) { + currentPhase = event.phase if (events.signal.aborted) return const current = phases.at(-1) if (current?.phase === event.phase) { @@ -42,23 +43,48 @@ export function generationStream( emit({ type: 'phase', data: phases }) } - async function run() { + async function sendResult(work: Promise, release: () => void) { try { - const result = await generate(progress) - if ( - Buffer.byteLength(JSON.stringify(result), 'utf8') > DEFAULT_GENERATION_LIMITS.maxOutputBytes - ) { + const result = await work + const bytes = Buffer.byteLength(JSON.stringify(result), 'utf8') + if (bytes > DEFAULT_GENERATION_LIMITS.maxOutputBytes) { throw new Error('Generated result is too large.') } finishPhase('completed') emit({ type: 'phase', data: phases }) emit({ type: 'done', data: { state: 'completed', result } }) - } catch { - // Provider exceptions may contain request details. Never send or log their raw text. - console.error('AI generation failed.') + } catch (cause) { + const validation = cause instanceof GenerationValidationError ? cause.message : undefined + const error = cause instanceof Error ? cause : undefined + const header = error ? `${error.name}: ${error.message}` : '' + const stack = error?.stack + const providerCause = error?.cause ?? cause + let status: number | undefined + if ( + providerCause && + typeof providerCause === 'object' && + 'status' in providerCause && + typeof providerCause.status === 'number' + ) { + status = providerCause.status + } + const callSite = stack?.startsWith(header) + ? stack.slice(header.length).trimStart() + : undefined + // Keep the call site and HTTP status, not a provider message that may echo inputs. + console.error('AI generation failed.', { + phase: currentPhase, + validation, + error: error?.name ?? 'Unknown error', + status, + stack: callSite + }) finishPhase('failed') emit({ type: 'phase', data: phases }) - emit({ type: 'error', data: { message: 'Generation failed. Please try again.' } }) + emit({ + type: 'error', + data: { message: validation ?? 'Generation failed. Please try again.' } + }) emit({ type: 'done', data: { state: 'failed' } }) } finally { // The core promise settles only after provider-file cleanup, even without a reader. @@ -69,7 +95,6 @@ export function generationStream( emit({ type: 'state', data: { state: 'running' } }) progress({ phase: 'preparation', message: 'Preparing generation' }) - // Accepted work runs even if the observer has already disconnected. - void run() - return events.response + // The request signal only detaches the observer; it never cancels accepted work. + return { response: events.response, progress, sendResult } } diff --git a/apps/web/src/lib/server/pieces.generation.test.ts b/apps/web/src/lib/server/pieces.generation.test.ts deleted file mode 100644 index a6ba5d3d..00000000 --- a/apps/web/src/lib/server/pieces.generation.test.ts +++ /dev/null @@ -1,44 +0,0 @@ -import { afterEach, expect, test, vi } from 'vitest' -import { pieceFrontMatterFromPrompt } from '@luzzle/core' -import type { PieceFrontmatter, PieceFrontmatterSchema } from '@luzzle/core' -import { promptToPiece } from './pieces' - -vi.mock('./config', () => ({ config: { ai: { api_key: 'test-key' } } })) -vi.mock('./storage', () => ({ getStorage: vi.fn() })) -vi.mock('@luzzle/core', async (importOriginal) => ({ - ...(await importOriginal>()), - pieceFrontMatterFromPrompt: vi.fn() -})) - -afterEach(() => { - vi.mocked(pieceFrontMatterFromPrompt).mockReset() -}) - -const schema = { - title: 'books', - type: 'object', - properties: { title: { type: 'string' } }, - required: ['title'] -} as PieceFrontmatterSchema - -test('web generation reports SDK aborts as timeouts', async () => { - vi.mocked(pieceFrontMatterFromPrompt).mockRejectedValueOnce( - new Error('exception AbortError: This operation was aborted sending request') - ) - - await expect(promptToPiece(schema, 'prompt')).rejects.toThrow( - 'A Gemini request timed out after 5 minutes' - ) -}) - -test('web generation preserves unrelated provider errors', async () => { - vi.mocked(pieceFrontMatterFromPrompt).mockRejectedValueOnce(new Error('quota exceeded')) - - await expect(promptToPiece(schema, 'prompt')).rejects.toThrow('quota exceeded') -}) - -test('web generation returns successful metadata', async () => { - vi.mocked(pieceFrontMatterFromPrompt).mockResolvedValueOnce({ title: 'Generated' }) - - await expect(promptToPiece(schema, 'prompt')).resolves.toEqual({ title: 'Generated' }) -}) diff --git a/apps/web/src/lib/server/pieces.test.ts b/apps/web/src/lib/server/pieces.test.ts index f54393ca..7324b943 100644 --- a/apps/web/src/lib/server/pieces.test.ts +++ b/apps/web/src/lib/server/pieces.test.ts @@ -2,7 +2,7 @@ import { describe, expect, test, vi } from 'vitest' import { createAppDb, runWebMigrations } from '@luzzle/web.db' import { getWebPiece, getRecentlyEditedPieces } from './pieces.js' -// pieces.js pulls in getPieces()/promptToPiece()'s module-level deps on import +// pieces.js pulls in getPieces()'s module-level dependencies on import vi.mock('$lib/server/config', () => ({ config: {} })) vi.mock('$lib/server/storage', () => ({ getStorage: vi.fn() })) diff --git a/apps/web/src/lib/server/pieces.ts b/apps/web/src/lib/server/pieces.ts index 8e07f707..59dfb709 100644 --- a/apps/web/src/lib/server/pieces.ts +++ b/apps/web/src/lib/server/pieces.ts @@ -1,13 +1,7 @@ -import { - Pieces, - pieceFrontMatterFromPrompt, - type PieceFrontmatter, - type PieceFrontmatterSchema -} from '@luzzle/core' +import { Pieces } from '@luzzle/core' import { sql, type Kysely } from 'kysely' import type { AppDatabase, WebPieces } from '@luzzle/web.db' import { getStorage } from './storage' -import { config } from './config' let pieces: Pieces | null = null @@ -22,25 +16,6 @@ export function getPieces(): Pieces { return pieces } -export async function promptToPiece( - schema: PieceFrontmatterSchema, - prompt: string, - file?: Buffer[] -) { - if (!config.ai) { - throw new Error('AI is not configured. Please check your config.yaml') - } - - try { - return await pieceFrontMatterFromPrompt(config.ai.api_key, schema, prompt, file) - } catch (error) { - if (error instanceof Error && error.message.includes('AbortError')) { - throw new Error('A Gemini request timed out after 5 minutes') - } - throw error - } -} - export async function getRecentPieces( db: Kysely, limit: number diff --git a/apps/web/src/lib/sse.test.ts b/apps/web/src/lib/sse.test.ts new file mode 100644 index 00000000..69205203 --- /dev/null +++ b/apps/web/src/lib/sse.test.ts @@ -0,0 +1,184 @@ +import { describe, expect, it, vi } from 'vitest' +import { readJsonEvents, type JsonEvent } from './sse' + +const encoder = new TextEncoder() + +function stream(text = '') { + let controller: ReadableStreamDefaultController + const cancel = vi.fn() + const body = new ReadableStream({ + start(value) { + controller = value + }, + cancel + }) + const bytes = (value: Uint8Array) => controller.enqueue(value) + if (text) bytes(encoder.encode(text)) + return { + response: new Response(body), + body, + cancel, + bytes, + close: () => controller.close(), + fail: (error: Error) => controller.error(error) + } +} + +async function collect(response: Response, signal = new AbortController().signal) { + const events: JsonEvent[] = [] + for await (const event of readJsonEvents(response, signal)) events.push(event) + return events +} + +describe('readJsonEvents', () => { + it.each([1, 7, 61])('decodes CRLF frames and UTF-8 split into %i-byte chunks', async (size) => { + const source = stream() + const bytes = encoder.encode( + ': heartbeat\r\n\r\nevent: update\r\ndata: {"text":"café 🧩 日本語"}\r\n\r\n' + + 'event: finished\r\ndata: true\r\n\r\n' + ) + for (let i = 0; i < bytes.length; i += size) source.bytes(bytes.slice(i, i + size)) + source.close() + await expect(collect(source.response)).resolves.toEqual([ + { type: 'update', data: { text: 'café 🧩 日本語' } }, + { type: 'finished', data: true } + ]) + expect(source.body.locked).toBe(false) + }) + + it.each(['\n', '\r\n', '\r'])( + 'handles multiline data, comments, and default events (%j)', + async (newline) => { + const source = stream( + [ + ': heartbeat', + '', + 'event: ignored-without-data', + '', + 'id: unused', + 'retry: 1000', + 'unknown: ignored', + 'data: {', + ': comment inside frame', + 'data: "value": 42}', + '', + 'event: first', + 'event: last', + 'data:null', + '', + 'event:', + 'data: "default"', + '', + '' + ].join(newline) + ) + source.close() + await expect(collect(source.response)).resolves.toEqual([ + { type: 'message', data: { value: 42 } }, + { type: 'last', data: null }, + { type: 'message', data: 'default' } + ]) + } + ) + + it('dispatches a CR-terminated frame without waiting for another chunk or EOF', async () => { + const source = stream('data: 1\r\r') + const events = readJsonEvents(source.response, new AbortController().signal) + await expect(events.next()).resolves.toEqual({ + done: false, + value: { type: 'message', data: 1 } + }) + await events.return(undefined) + expect(source.cancel).toHaveBeenCalledOnce() + await vi.waitFor(() => expect(source.body.locked).toBe(false)) + }) + + it('does not dispatch an unterminated frame at EOF', async () => { + const source = stream('data: 1\n\ndata: 2\n') + source.close() + await expect(collect(source.response)).resolves.toEqual([{ type: 'message', data: 1 }]) + expect(source.body.locked).toBe(false) + }) + + it('cancels and releases the reader when the consumer stops early', async () => { + const source = stream('data: 1\n\ndata: 2\n\n') + for await (const event of readJsonEvents(source.response, new AbortController().signal)) { + expect(event.data).toBe(1) + break + } + expect(source.cancel).toHaveBeenCalledOnce() + await vi.waitFor(() => expect(source.body.locked).toBe(false)) + }) + + it.each(['data: {broken\n\n', 'data\n\n'])( + 'rejects invalid JSON and releases the reader (%#)', + async (text) => { + const source = stream(text) + await expect(collect(source.response)).rejects.toThrow('Invalid SSE event data.') + expect(source.cancel).toHaveBeenCalledOnce() + expect(source.body.locked).toBe(false) + } + ) + + it.each([false, true])('rejects invalid or incomplete UTF-8 (EOF: %s)', async (eof) => { + const source = stream() + source.bytes(new Uint8Array(eof ? [0xc3] : [0xff])) + if (eof) source.close() + await expect(collect(source.response)).rejects.toBeInstanceOf(TypeError) + expect(source.body.locked).toBe(false) + }) + + it('aborts a pending read cleanly', async () => { + const source = stream() + const observer = new AbortController() + const pending = collect(source.response, observer.signal) + expect(source.body.locked).toBe(true) + observer.abort() + await expect(pending).resolves.toEqual([]) + expect(source.cancel).toHaveBeenCalledOnce() + expect(source.body.locked).toBe(false) + }) + + it('closes a response without reading when already aborted', async () => { + const source = stream('data: 1\n\n') + await expect(collect(source.response, AbortSignal.abort())).resolves.toEqual([]) + expect(source.cancel).toHaveBeenCalledOnce() + expect(source.body.locked).toBe(false) + }) + + it('discards buffered events after abort', async () => { + const source = stream('data: 1\n\ndata: 2\n\n') + const observer = new AbortController() + const events: JsonEvent[] = [] + for await (const event of readJsonEvents(source.response, observer.signal)) { + events.push(event) + observer.abort() + } + expect(events).toEqual([{ type: 'message', data: 1 }]) + expect(source.cancel).toHaveBeenCalledOnce() + await vi.waitFor(() => expect(source.body.locked).toBe(false)) + }) + + it('propagates a read failure and releases the reader', async () => { + const source = stream() + const pending = collect(source.response) + source.fail(new Error('Connection lost')) + await expect(pending).rejects.toThrow('Connection lost') + expect(source.body.locked).toBe(false) + }) + + it('releases the reader even if cancellation fails', async () => { + const source = stream('data: 1\n\n') + source.cancel.mockRejectedValue(new Error('Already disconnected')) + for await (const event of readJsonEvents(source.response, new AbortController().signal)) { + expect(event.data).toBe(1) + break + } + await vi.waitFor(() => expect(source.body.locked).toBe(false)) + }) + + it('rejects a missing response body unless observation was aborted', async () => { + await expect(collect(new Response(null))).rejects.toThrow('no stream') + await expect(collect(new Response(null), AbortSignal.abort())).resolves.toEqual([]) + }) +}) diff --git a/apps/web/src/lib/sse.ts b/apps/web/src/lib/sse.ts new file mode 100644 index 00000000..8b164593 --- /dev/null +++ b/apps/web/src/lib/sse.ts @@ -0,0 +1,42 @@ +import { EventSourceParserStream, type EventSourceMessage } from 'eventsource-parser/stream' + +export type JsonEvent = { type: string; data: unknown } + +function parseJsonEvent({ event, data }: EventSourceMessage): JsonEvent { + try { + return { type: event || 'message', data: JSON.parse(data) } + } catch { + throw new Error('Invalid SSE event data.') + } +} + +/** Read JSON SSE frames. Aborting stops observation, not the server's work. */ +export async function* readJsonEvents( + response: Response, + signal: AbortSignal +): AsyncGenerator { + if (!response.body) { + if (signal.aborted) return + throw new Error('SSE response has no stream.') + } + if (signal.aborted) { + await response.body.cancel().catch(() => {}) + return + } + const events = response.body + .pipeThrough(new TextDecoderStream('utf-8', { fatal: true })) + .pipeThrough(new EventSourceParserStream(), { signal }) + const reader = events.getReader() + try { + while (!signal.aborted) { + const { value, done } = await reader.read() + if (done || signal.aborted) return + yield parseJsonEvent(value) + } + } catch (cause) { + if (!signal.aborted) throw cause + } finally { + await reader.cancel().catch(() => {}) + reader.releaseLock() + } +} diff --git a/apps/web/src/routes/admin/piece/[...path]/preview/[jobId]/WorkflowConsole.svelte b/apps/web/src/routes/admin/piece/[...path]/preview/[jobId]/WorkflowConsole.svelte index e6aaf1e2..97afce78 100644 --- a/apps/web/src/routes/admin/piece/[...path]/preview/[jobId]/WorkflowConsole.svelte +++ b/apps/web/src/routes/admin/piece/[...path]/preview/[jobId]/WorkflowConsole.svelte @@ -1,6 +1,12 @@ - +
+ + {#if status === 'completed'} + + {:else if status === 'running'} + + {:else if status === 'enqueued'} + + {:else if status === 'failed' || status === 'expired'} + + {:else} + + {/if} + + +
+ {#if status === 'expired'} +
Preview expired
+
+ Assets are purged after 2 days. Re-run the preview from the editor. +
+ {:else if status === 'enqueued'} +
Enqueued
+
Waiting for worker…
+ {:else if status === 'running'} +
Rendering preview
+
{formatDuration(overallDuration)} elapsed
+ {:else if status === 'failed'} +
Preview failed
+
{formatDuration(overallDuration)} stopped at
+ {:else if status === 'completed'} +
Preview completed
+
{formatDuration(overallDuration)} total time
+ {/if} +
+
+ +{#if errorMessage && status === 'failed'} +
+ + {errorMessage} +
+{/if} + +{#if phases.length > 0} +
+ {#each phases as phase (phase.phase)} + {@const hasLogs = (logs[phase.phase]?.length ?? 0) > 0} +
+
+ + {#if phase.status === 'completed'} + + {:else if phase.status === 'running'} + + {:else if phase.status === 'failed'} + + {:else if phase.status === 'skipped'} + + {:else} + + {/if} + + {phase.phase} + {#if phase.message} + + {phase.message} + + {/if} + + {#if phase.started_at} + + {formatDuration((phase.finished_at || Date.now()) - phase.started_at)} + + {/if} + +
+ + {#if hasLogs} +
+
+ {logs[phase.phase]?.length || 0} lines +
+ {#if phase.started_at} + + {formatDuration((phase.finished_at || Date.now()) - phase.started_at)} + + {/if} + +
+
+
handleScroll(phase.phase, e)} + > + {#each logs[phase.phase] ?? [] as log (log.line_number)} +
+ + {new Date(log.ts).toISOString().split('T')[1].slice(0, -5)} + + {log.message} +
+ {/each} +
+
+ {/if} +
+ {/each} +
+{/if} + + diff --git a/apps/web/src/lib/components/ProgressConsole.test.ts b/apps/web/src/routes/admin/piece/[...path]/preview/[jobId]/WorkflowConsole.test.ts similarity index 59% rename from apps/web/src/lib/components/ProgressConsole.test.ts rename to apps/web/src/routes/admin/piece/[...path]/preview/[jobId]/WorkflowConsole.test.ts index 9ce8bd45..dcdc2179 100644 --- a/apps/web/src/lib/components/ProgressConsole.test.ts +++ b/apps/web/src/routes/admin/piece/[...path]/preview/[jobId]/WorkflowConsole.test.ts @@ -1,8 +1,7 @@ import { afterEach, beforeEach, describe, expect, test, vi } from 'vitest' import { render } from 'svelte/server' -import ProgressConsole from './ProgressConsole.svelte' -import WorkflowConsole from '../../routes/admin/piece/[...path]/preview/[jobId]/WorkflowConsole.svelte' -import type { ProgressPhase, ProgressLog } from './progress.js' +import WorkflowConsole from './WorkflowConsole.svelte' +import type { ProgressPhase, ProgressLog } from '$lib/components/progress.js' const started = Date.parse('2026-01-02T03:04:00Z') const phases: ProgressPhase[] = [ @@ -32,7 +31,7 @@ beforeEach(() => { }) afterEach(() => vi.restoreAllMocks()) -describe('preview compatibility wrapper', () => { +describe('preview status summary', () => { test.each([ [ 'expired', @@ -72,23 +71,62 @@ describe('preview compatibility wrapper', () => { ) }) -describe('shared progress presentation', () => { - test('accepts caller wording and progress/log rows without job IDs', () => { - const result = render(ProgressConsole, { +describe('preview phases and logs', () => { + test('renders preview logs with auto-scroll locked by default', () => { + const result = render(WorkflowConsole, { props: { status: 'completed', - statusText: { title: 'Content ready', durationLabel: 'finished in' }, phases, logs } }) - expect(text(result.body)).toContain('Content ready 1m 5s finished in') - expect(text(result.body)).not.toContain('Preview') - expect(text(result.body)).not.toContain('worker') + expect(text(result.body)).toContain('Preview completed 1m 5s total time') expect(text(result.body)).toContain('1 lines') expect(result.body).toContain('Auto-scroll locked') }) + test('omits the timeline without phases and log panels without logs', () => { + const empty = render(WorkflowConsole, { + props: { status: 'enqueued', phases: [], logs: {} } + }) + expect(empty.body).not.toContain('class="timeline') + const withoutLogs = render(WorkflowConsole, { + props: { status: 'completed', phases, logs: {} } + }) + expect(withoutLogs.body).toContain('class="timeline') + expect(withoutLogs.body).not.toContain('class="log-console') + }) + + test('scopes log viewport IDs to the rendered console', () => { + const props = { status: 'completed', phases, logs } + const first = render(WorkflowConsole, { props, idPrefix: 'first' }) + const second = render(WorkflowConsole, { props, idPrefix: 'second' }) + const viewportId = (body: string) => body.match(/id="([^"]+-log-container-render)"/)?.[1] + expect(viewportId(first.body)).toBeTruthy() + expect(viewportId(second.body)).toBeTruthy() + expect(viewportId(first.body)).not.toBe(viewportId(second.body)) + }) + + test('measures total preview duration from the first phase through the last', () => { + const result = render(WorkflowConsole, { + props: { + status: 'completed', + phases: [ + phases[0], + { + ...phases[0], + phase: 'publish', + started_at: started + 65_000, + finished_at: started + 90_000 + } + ], + logs: {} + } + }) + expect(text(result.body)).toContain('Preview completed 1m 30s total time') + expect(text(result.body)).toContain('25s') + }) + test.each([ [[], '0s'], [[{ ...phases[0], started_at: 0 }], '0s'], @@ -96,15 +134,14 @@ describe('shared progress presentation', () => { [[{ ...phases[0], finished_at: started + 5000 }], '5s'], [[{ ...phases[0], finished_at: started - 1000 }], '0s'] ])('retains elapsed-time formatting for %j', (items, duration) => { - const result = render(ProgressConsole, { + const result = render(WorkflowConsole, { props: { status: 'running', - statusText: { title: 'Working', durationLabel: 'elapsed' }, phases: items, logs: {} } }) - expect(text(result.body)).toContain(`Working ${duration} elapsed`) + expect(text(result.body)).toContain(`Rendering preview ${duration} elapsed`) }) test('retains phase statuses and stderr/error highlighting', () => { @@ -113,7 +150,7 @@ describe('shared progress presentation', () => { phase: status, status })) - const result = render(ProgressConsole, { + const result = render(WorkflowConsole, { props: { status: 'failed', phases: items, @@ -130,12 +167,27 @@ describe('shared progress presentation', () => { expect(text(result.body)).toContain('Failure details') }) - test('escapes caller titles, messages, and logs', () => { + test.each(['stdout', 'info', 'stderr', 'error'])( + 'highlights only error log levels: %s', + (level) => { + const result = render(WorkflowConsole, { + props: { + status: 'completed', + phases, + logs: { render: [{ ...logs.render[0], level }] } + } + }) + const rowClasses = result.body.match(/class="(log-row[^"]*)"/)?.[1] + expect(rowClasses).toBeTruthy() + expect(rowClasses?.includes('is-error')).toBe(level === 'stderr' || level === 'error') + } + ) + + test('escapes preview errors, phase messages, and logs', () => { const unsafe = '' - const result = render(ProgressConsole, { + const result = render(WorkflowConsole, { props: { status: 'failed', - statusText: { title: unsafe, description: unsafe }, errorMessage: unsafe, phases: [{ ...phases[0], message: unsafe }], logs: { render: [{ ...logs.render[0], message: unsafe }] } diff --git a/apps/web/src/routes/admin/piece/[...path]/source/+page.server.ts b/apps/web/src/routes/admin/piece/[...path]/source/+page.server.ts index 838b0b57..8157cbea 100644 --- a/apps/web/src/routes/admin/piece/[...path]/source/+page.server.ts +++ b/apps/web/src/routes/admin/piece/[...path]/source/+page.server.ts @@ -33,7 +33,7 @@ async function resolveAssetUrls( for (const instancePath of instancePaths) { const value = getFrontmatterValue(updated.frontmatter, instancePath) - if (typeof value !== 'string' || !/^https?:\/\//.test(value)) continue + if (typeof value !== 'string' || !/^https?:\/\//i.test(value)) continue try { const parts = instancePath.split('.') diff --git a/apps/web/src/routes/admin/piece/[...path]/source/+page.svelte b/apps/web/src/routes/admin/piece/[...path]/source/+page.svelte index 88e86298..debbac7d 100644 --- a/apps/web/src/routes/admin/piece/[...path]/source/+page.svelte +++ b/apps/web/src/routes/admin/piece/[...path]/source/+page.svelte @@ -3,13 +3,14 @@ import MarkdownEditor from '$lib/components/editor/MarkdownEditor.svelte' import Button from '$lib/components/ui/Button.svelte' import PieceActions from '$lib/components/editor/PieceActions.svelte' + import GenerationDialog from '$lib/components/editor/GenerationDialog.svelte' import type { PageProps } from './$types' import { Dialog } from 'bits-ui' import { fade, fly } from 'svelte/transition' import { page } from '$app/state' - import { beforeNavigate, goto } from '$app/navigation' - import { onMount } from 'svelte' + import { beforeNavigate, goto, replaceState } from '$app/navigation' + import { onMount, onDestroy } from 'svelte' let { data, form }: PageProps = $props() @@ -18,6 +19,11 @@ let bypassWarning = $state(false) let isSaving = $state(false) let saveSuccess = $state(false) + let editorError = $state('') + let disposed = false + onDestroy(() => { + disposed = true + }) beforeNavigate((navigation) => { if (isDirty && !bypassWarning && navigation.type !== 'form' && navigation.to) { @@ -68,6 +74,22 @@ } }) + let generateDialogOpen = $state(false) + $effect(() => { + if (data.file) { + generateDialogOpen = false + editorError = '' + } + }) + $effect(() => { + if (page.url.searchParams.get('generate') === '1') { + generateDialogOpen = data.canGenerate + const url = new URL(page.url) + url.searchParams.delete('generate') + replaceState(url, page.state) + } + }) + let attachDialogOpen = $state(false) let activeTab = $state<'file' | 'url' | 'create'>('file') let selectedFile = $state(null) @@ -171,6 +193,26 @@ } +{#key data.file} + key !== '__proto__')} + onFocusEditor={() => editorRef?.focus()} + onError={(message) => { + editorError = message + saveSuccess = false + }} + onApply={(source, markdown) => { + if (!editorRef) throw new Error('The editor is not ready. Please try again.') + editorRef.replaceContent(markdown, source) + saveSuccess = false + editorError = '' + }} + /> +{/key} + { @@ -383,13 +425,25 @@ isSaving = true bypassWarning = true saveSuccess = false + editorError = '' return async ({ result, update }) => { - isSaving = false - bypassWarning = false - if (result.type === 'success') { - saveSuccess = true + if (disposed) return + try { + if (result.type === 'redirect') { + editorError = + 'Please sign in again in another tab, then retry. Your edits are still here.' + return + } + if (result.type === 'error') { + editorError = 'Unable to save. Your edits are still here. Please try again.' + return + } + if (result.type === 'success') saveSuccess = true + await update({ reset: false }) + } finally { + isSaving = false + bypassWarning = false } - await update({ reset: false }) } }} > @@ -406,10 +460,14 @@ file={data.file} currentMode="source" {isDirty} - canGenerate={data.canGenerate} + canGenerate={data.canGenerate && !isSaving} {hasPublicVersion} {isPublicStale} onDelete={() => dialog.showModal()} + onGenerate={() => { + editorError = '' + generateDialogOpen = true + }} onAttach={() => { const selectedText = editorRef?.getSelectedText()?.trim() || '' let isUrl = false @@ -437,10 +495,10 @@ {/if} - {#if form?.error} -