diff --git a/shared/applyParameterSpecs.test.ts b/shared/applyParameterSpecs.test.ts new file mode 100644 index 00000000..31f1eb7d --- /dev/null +++ b/shared/applyParameterSpecs.test.ts @@ -0,0 +1,200 @@ +import assert from 'node:assert/strict'; +import { describe, it } from 'node:test'; +import applyParameterSpecs from './applyParameterSpecs.ts'; +import parseParameters from './parseParameters.ts'; +import type { ParameterSpec } from './types.ts'; + +const CODE = ` +cup_height = 100; // [50:5:200] +style = "round"; +size = [30, 20, 10]; + +module body() {} +`; + +describe('applyParameterSpecs', () => { + it('returns parsed parameters untouched when no specs are given', () => { + const parsed = parseParameters(CODE); + assert.deepEqual(applyParameterSpecs(parsed, undefined), parsed); + assert.deepEqual(applyParameterSpecs(parsed, null), parsed); + assert.deepEqual(applyParameterSpecs(parsed, []), parsed); + }); + + it('overlays spec metadata over comment-derived metadata', () => { + const specs: ParameterSpec[] = [ + { + name: 'cup_height', + label: 'Cup Height', + description: 'Outer height of the mug body', + group: 'Body', + min: 60, + max: 180, + step: 10, + unit: 'mm', + }, + ]; + const [cupHeight] = applyParameterSpecs(parseParameters(CODE), specs); + assert.equal(cupHeight.name, 'cup_height'); + assert.equal(cupHeight.displayName, 'Cup Height'); + assert.equal(cupHeight.description, 'Outer height of the mug body'); + assert.equal(cupHeight.group, 'Body'); + assert.equal(cupHeight.unit, 'mm'); + // Spec range wins over the `// [50:5:200]` Customizer comment. + assert.deepEqual(cupHeight.range, { min: 60, max: 180, step: 10 }); + // Value and default still come from the code. + assert.equal(cupHeight.value, 100); + assert.equal(cupHeight.defaultValue, 100); + }); + + it('drops spec entries that reference variables missing from the code', () => { + const parsed = parseParameters(CODE); + const result = applyParameterSpecs(parsed, [ + { name: 'phantom_variable', min: 0, max: 1 }, + ]); + assert.deepEqual(result, parsed); + }); + + it('keeps comment-derived metadata for parameters without a spec', () => { + const result = applyParameterSpecs(parseParameters(CODE), [ + { name: 'style', options: [{ value: 'round' }, { value: 'hex' }] }, + ]); + const cupHeight = result.find((param) => param.name === 'cup_height'); + assert.deepEqual(cupHeight?.range, { min: 50, step: 5, max: 200 }); + }); + + it('applies enum options, coercing values for number parameters', () => { + const result = applyParameterSpecs(parseParameters(CODE), [ + { + name: 'style', + options: [{ value: 'round', label: 'Round' }, { value: 'hex' }], + }, + { + name: 'cup_height', + options: [{ value: '80' }, { value: '120' }, { value: 'tall' }], + }, + ]); + const style = result.find((param) => param.name === 'style'); + assert.deepEqual(style?.options, [ + { value: 'round', label: 'Round' }, + { value: 'hex', label: 'hex' }, + ]); + const cupHeight = result.find((param) => param.name === 'cup_height'); + // 'tall' is not a number — coerced out rather than corrupting the list. + assert.deepEqual(cupHeight?.options, [ + { value: 80, label: '80' }, + { value: 120, label: '120' }, + ]); + }); + + it('spreads a base-name spec across flattened vector components', () => { + const result = applyParameterSpecs(parseParameters(CODE), [ + { name: 'size', min: 5, max: 50, unit: 'mm', label: 'Size' }, + ]); + const components = result.filter((param) => param.name.startsWith('size[')); + assert.equal(components.length, 3); + for (const component of components) { + assert.deepEqual(component.range, { min: 5, max: 50 }); + assert.equal(component.unit, 'mm'); + } + // Axis labels stay distinct — the shared label must not collapse them. + assert.notEqual(components[0].displayName, components[1].displayName); + }); + + it('expands a bound that excludes the current code value', () => { + // cup_height is 100 in the code — a min-only spec above it would pin + // the slider outside its own value and rewrite geometry on first drag. + const result = applyParameterSpecs(parseParameters(CODE), [ + { name: 'cup_height', min: 150 }, + ]); + const cupHeight = result.find((param) => param.name === 'cup_height'); + assert.equal(cupHeight?.range?.min, 100); + + const shrunk = applyParameterSpecs(parseParameters(CODE), [ + { name: 'cup_height', min: 20, max: 60 }, + ]); + assert.deepEqual( + shrunk.find((param) => param.name === 'cup_height')?.range, + { min: 20, max: 100, step: 5 }, + ); + }); + + it('rejects a zero-span range (min === max)', () => { + const result = applyParameterSpecs(parseParameters(CODE), [ + { name: 'cup_height', min: 100, max: 100 }, + ]); + const cupHeight = result.find((param) => param.name === 'cup_height'); + assert.deepEqual(cupHeight?.range, { min: 50, step: 5, max: 200 }); + }); + + it('ignores numeric range specs on string parameters', () => { + const result = applyParameterSpecs(parseParameters(CODE), [ + { name: 'style', min: 1, max: 10 }, + ]); + const style = result.find((param) => param.name === 'style'); + // range.max means maxLength for strings — a numeric range spec on a + // string variable must not leak into it. + assert.deepEqual(style?.range, {}); + }); + + it('ignores non-string presentation fields from unvalidated persisted parts', () => { + const parsed = parseParameters(CODE); + const result = applyParameterSpecs(parsed, [ + { + name: 'cup_height', + label: { nested: true }, + description: 42, + group: [], + unit: { mm: true }, + }, + ] as unknown as ParameterSpec[]); + const cupHeight = result.find((param) => param.name === 'cup_height'); + const original = parsed.find((param) => param.name === 'cup_height'); + assert.equal(cupHeight?.displayName, original?.displayName); + assert.equal(cupHeight?.description, original?.description); + assert.equal(cupHeight?.unit, undefined); + }); + + it('caps unit length so it cannot blow up the layout', () => { + const result = applyParameterSpecs(parseParameters(CODE), [ + { name: 'cup_height', unit: ' millimeters ' }, + ]); + assert.equal( + result.find((param) => param.name === 'cup_height')?.unit, + 'mill', + ); + }); + + it('rejects option values with unit suffixes for number parameters', () => { + const result = applyParameterSpecs(parseParameters(CODE), [ + { name: 'cup_height', options: [{ value: '80mm' }, { value: '120' }] }, + ]); + // parseFloat would read "80mm" as 80 — Number() must reject it. + assert.deepEqual( + result.find((param) => param.name === 'cup_height')?.options, + [{ value: 120, label: '120' }], + ); + }); + + it('rejects inverted ranges and non-positive steps from a bad spec', () => { + const result = applyParameterSpecs(parseParameters(CODE), [ + { name: 'cup_height', min: 200, max: 50, step: -5 }, + ]); + const cupHeight = result.find((param) => param.name === 'cup_height'); + // Falls back to the comment-derived range instead of breaking sliders. + assert.deepEqual(cupHeight?.range, { min: 50, step: 5, max: 200 }); + }); + + it('tolerates malformed spec entries from an untrusted tool input', () => { + const parsed = parseParameters(CODE); + const result = applyParameterSpecs(parsed, [ + null, + 42, + { noName: true }, + { name: 'style', options: [null, { value: 7 }] }, + ] as unknown as ParameterSpec[]); + assert.equal(result.length, parsed.length); + const style = result.find((param) => param.name === 'style'); + // No usable options survive validation — comment-derived (empty) kept. + assert.deepEqual(style?.options, []); + }); +}); diff --git a/shared/applyParameterSpecs.ts b/shared/applyParameterSpecs.ts new file mode 100644 index 00000000..7e5aa564 --- /dev/null +++ b/shared/applyParameterSpecs.ts @@ -0,0 +1,160 @@ +import type { Parameter, ParameterOption, ParameterSpec } from './types.ts'; + +/** + * Overlay the model-authored parameter schema (`artifact.parameters`) onto + * the parameters parsed from the OpenSCAD source. + * + * The code stays the ground truth for which parameters EXIST and what value + * they currently hold — `parseParameters` extracts those. The structured + * schema is the source of truth for presentation metadata: ranges, units, + * labels, descriptions, groups, and enum options. + * + * The parse doubles as the validator on the schema: + * - spec entries that don't match a variable in the code are ignored, + * - nonsensical ranges (min > max, step <= 0) fall back to comment-derived + * metadata instead of breaking the sliders, + * - parameters without a spec keep their comment-derived metadata, + * so artifacts persisted before the schema existed — and models that omit + * it — render exactly as before. + */ +// Caps on model- and DB-sourced metadata. Persisted parts re-enter without +// zod validation (see `asParametricParts`), and ShareView renders other +// people's artifacts — unbounded specs would let one malicious shared link +// burn every viewer's memory and render time. +const MAX_SPECS = 100; +const MAX_OPTIONS = 50; +const MAX_LABEL_LENGTH = 80; +const MAX_DESCRIPTION_LENGTH = 300; +const MAX_GROUP_LENGTH = 60; +const MAX_OPTION_LENGTH = 120; +const MAX_UNIT_LENGTH = 4; +// Bounds beyond any physical CAD dimension (1e9 mm = 1000 km) are noise, +// and extreme magnitudes (1e308, 5e-324) break the slider's percent and +// decimal math downstream. +const MAX_BOUND_MAGNITUDE = 1e9; +const MIN_STEP = 1e-9; + +export default function applyParameterSpecs( + parameters: Parameter[], + specs: ParameterSpec[] | null | undefined, +): Parameter[] { + if (!Array.isArray(specs) || specs.length === 0) return parameters; + + const byName = new Map(); + for (const spec of specs.slice(0, MAX_SPECS)) { + if ( + spec && + typeof spec === 'object' && + typeof spec.name === 'string' && + !byName.has(spec.name) + ) { + byName.set(spec.name, spec); + } + } + + return parameters.map((parameter) => { + // `size[2]`-style names come from flattened vector declarations — fall + // back to a spec on the base vector name so one `size` entry covers all + // of its components. + const baseName = parameter.name.replace(/\[\d+\]$/, ''); + const exact = byName.get(parameter.name); + const spec = exact ?? byName.get(baseName); + if (!spec) return parameter; + + const merged: Parameter = { ...parameter }; + + // Every field is re-checked with `typeof` even though the live tool + // path is zod-validated: persisted message parts re-enter through + // `asParametricParts`, which does NOT validate `parameters`, and a + // non-string label/description would crash React as a child node — + // taking down ShareView for every viewer of a shared link. + // + // Vector components matched via the base name keep their derived axis + // label ("Body Width") — sharing one label across every axis would make + // the sliders indistinguishable. + if (typeof exact?.label === 'string' && exact.label) { + merged.displayName = exact.label.slice(0, MAX_LABEL_LENGTH); + } + if (typeof spec.description === 'string' && spec.description) { + merged.description = spec.description.slice(0, MAX_DESCRIPTION_LENGTH); + } + if (typeof spec.group === 'string' && spec.group) { + merged.group = spec.group.slice(0, MAX_GROUP_LENGTH); + } + if (typeof spec.unit === 'string' && spec.unit.trim()) { + // The unit renders in a fixed ~4-character span next to the value — + // cap it so a runaway model string can't blow up the layout. + merged.unit = spec.unit.trim().slice(0, MAX_UNIT_LENGTH); + } + + // Numeric bounds only apply to number parameters. For strings the + // parse-land `range.max` doubles as maxLength, so a numeric range spec + // on a string variable is a contradiction, not an instruction. + if (parameter.type === 'number' || parameter.type === undefined) { + const range = { ...parameter.range }; + if (typeof spec.min === 'number' && isSaneBound(spec.min)) { + range.min = spec.min; + } + if (typeof spec.max === 'number' && isSaneBound(spec.max)) { + range.max = spec.max; + } + if ( + typeof spec.step === 'number' && + isSaneBound(spec.step) && + spec.step >= MIN_STEP + ) { + range.step = spec.step; + } + // The code's current value is ground truth — a bound that excludes it + // would render a slider pinned outside its own value (and the next + // track interaction would silently rewrite the geometry), so expand + // the offending bound to contain it. + if (typeof parameter.value === 'number') { + if (range.min !== undefined && range.min > parameter.value) { + range.min = parameter.value; + } + if (range.max !== undefined && range.max < parameter.value) { + range.max = parameter.value; + } + } + // `>=`: a zero-span range NaNs the slider's percent math. + const degenerate = + range.min !== undefined && + range.max !== undefined && + range.min >= range.max; + merged.range = degenerate ? parameter.range : range; + } + + if (Array.isArray(spec.options) && spec.options.length > 0) { + const options: ParameterOption[] = []; + const seenValues = new Set(); + for (const option of spec.options.slice(0, MAX_OPTIONS)) { + if (!option || typeof option !== 'object') continue; + const raw = (option as { value?: unknown }).value; + if (typeof raw !== 'string' || raw.length === 0) continue; + if (raw.length > MAX_OPTION_LENGTH) continue; + // Duplicate values would render duplicate select entries (and + // duplicate React keys) — first occurrence wins. + if (seenValues.has(raw)) continue; + seenValues.add(raw); + // `Number`, not `parseFloat` — "80mm" must be rejected, not read + // as 80 and silently diverge from what the code would accept. + const value = parameter.type === 'number' ? Number(raw) : raw; + if (typeof value === 'number' && !Number.isFinite(value)) continue; + const label = + typeof option.label === 'string' && option.label + ? option.label.slice(0, MAX_OPTION_LENGTH) + : raw; + options.push({ value, label }); + } + if (options.length > 0) merged.options = options; + } + + return merged; + }); +} + +// Finite and within physical-CAD magnitude — see MAX_BOUND_MAGNITUDE. +function isSaneBound(value: number): boolean { + return Number.isFinite(value) && Math.abs(value) <= MAX_BOUND_MAGNITUDE; +} diff --git a/shared/chatAi.ts b/shared/chatAi.ts index 439a9cc3..84fb07e9 100644 --- a/shared/chatAi.ts +++ b/shared/chatAi.ts @@ -16,10 +16,99 @@ export const createMeshOutputSchema = z.object({ fileType: z.enum(['glb', 'stl', 'obj', 'fbx']), }); +/** + * A malformed `parameters` entry must never fail tool-input validation: + * an invalid input turns the whole build_parametric_model call into a + * `dynamic-tool` part the client never compiles, silently killing the + * build turn. So every field degrades instead of erroring: + * - numeric fields accept `"50"` (models routinely quote numbers) and + * drop to undefined on anything else (`null`, `""`, `1e309`, objects), + * - string fields drop to undefined on non-strings, + * - a spec entry with a broken `name` collapses to null, which the + * merge layer (`applyParameterSpecs`) skips. + * `.catch`/`.preprocess` don't change the JSON schema advertised to + * providers — they only relax what we accept back. + */ +const lenientNumber = z.preprocess( + (value) => + typeof value === 'string' && value.trim() !== '' ? Number(value) : value, + z.number().finite().optional(), +); + +export const parameterSpecSchema = z.object({ + name: z + .string() + .min(1) + .describe( + 'Exact name of the top-of-file OpenSCAD variable this entry describes.', + ), + label: z + .string() + .optional() + .catch(undefined) + .describe( + 'Display name shown in the parameter panel. Defaults to a title-cased variable name.', + ), + type: z.enum(['number', 'string', 'boolean']).optional().catch(undefined), + description: z + .string() + .optional() + .catch(undefined) + .describe('One short sentence on what the parameter controls.'), + group: z + .string() + .optional() + .catch(undefined) + .describe('Section heading grouping related parameters.'), + min: lenientNumber + .catch(undefined) + .describe('Slider minimum for number parameters.'), + max: lenientNumber + .catch(undefined) + .describe('Slider maximum for number parameters.'), + step: lenientNumber + .catch(undefined) + .describe('Slider step for number parameters.'), + unit: z + .string() + .optional() + .catch(undefined) + .describe('Physical unit such as "mm" or "deg". Omit for unitless values.'), + options: z + .array( + z.object({ + // Accept numbers too (models routinely emit `value: 80` for + // numeric enums) and normalize to string — the merge layer + // re-types against the parsed parameter. + value: z + .union([z.string(), z.number()]) + .transform((value) => String(value)) + .describe( + 'Option value exactly as it appears in the OpenSCAD source.', + ), + label: z.string().optional().catch(undefined), + }), + ) + .optional() + .catch(undefined) + .describe('Allowed values for enum-style parameters.'), +}); + export const parametricArtifactSchema = z.object({ title: z.string().min(1), version: z.string().default('v1'), code: z.string().min(20), + // Nullish (not just optional) so a model that emits an explicit `null` + // doesn't fail tool-input validation and stall the agent loop, and + // `.catch(null)` so a structurally broken array degrades to the + // comment-derived fallback instead of killing the build call. + parameters: z + .array(parameterSpecSchema) + .nullish() + .catch(null) + .describe( + 'Typed schema for every user-editable variable declared at the top of `code`. This is the source of truth for the parameter panel: ranges, units, enum options, grouping, descriptions. Every entry must reference a variable that exists in the code.', + ), }); export const parametricCompileOutputSchema = z.object({ @@ -42,7 +131,7 @@ export const answerUserSchema = z.object({ export const chatTools = { build_parametric_model: tool({ description: - 'Create or update the complete OpenSCAD CAD artifact. After the browser compiles it, inspect the returned multi-view preview sheet and call this tool again if the model needs another revision.', + 'Create or update the complete OpenSCAD CAD artifact, including the typed parameter schema that drives the parameter panel. After the browser compiles it, inspect the returned multi-view preview sheet and call this tool again if the model needs another revision.', inputSchema: parametricArtifactSchema, outputSchema: parametricCompileOutputSchema, }), diff --git a/shared/parametricParts.ts b/shared/parametricParts.ts index 388e1334..c1f26d1e 100644 --- a/shared/parametricParts.ts +++ b/shared/parametricParts.ts @@ -177,9 +177,10 @@ export function isParametricArtifact( return false; } const artifact = value as Partial; - // Title + code are the only load-bearing fields. `version` is metadata - // and `parts` is optional. Parameters are derived client-side from - // `code` via `parseParameters` so we don't check for them here either. + // Title + code are the only load-bearing fields. `version` is metadata. + // `parameters` is optional overlay metadata — parameter existence and + // values are still derived from `code` via `parseParameters`, with the + // schema applied on top by `applyParameterSpecs` — so we don't check it. return ( typeof artifact.title === 'string' && typeof artifact.code === 'string' ); diff --git a/shared/types.ts b/shared/types.ts index 039f895e..a32f8b84 100644 --- a/shared/types.ts +++ b/shared/types.ts @@ -39,6 +39,41 @@ export type ParametricArtifact = { title: string; version: string; code: string; + // Model-authored typed interface for the top-of-file variables in `code`. + // Optional (and nullish-tolerant) so artifacts persisted before the schema + // existed — and models that omit it — keep working via the Customizer + // comment fallback in `parseParameters`. See `applyParameterSpecs`. + parameters?: ParameterSpec[] | null; +}; + +/** + * One entry of the structured parameter schema the model emits alongside + * the OpenSCAD source in `build_parametric_model`. The code remains the + * ground truth for which variables exist and their current values; specs + * carry the presentation metadata (ranges, units, labels, groups, enum + * options) that previously had to be reverse-engineered from Customizer + * comments by regex. + */ +export type ParameterSpec = { + /** Exact name of the top-of-file OpenSCAD variable this spec describes. */ + name: string; + /** Display name for the parameter panel; defaults to a title-cased name. */ + label?: string; + type?: 'number' | 'string' | 'boolean'; + description?: string; + /** Section heading grouping related parameters. */ + group?: string; + min?: number; + max?: number; + step?: number; + /** Physical unit shown next to the value, e.g. "mm" or "deg". */ + unit?: string; + /** + * Allowed values for enum-style parameters. Values are strings exactly as + * they appear in the source (numbers as plain strings) — kept union-free + * so every provider's JSON-schema subset accepts the tool definition. + */ + options?: { value: string; label?: string }[]; }; export type ParameterOption = { value: string | number; label: string }; @@ -65,6 +100,8 @@ export type Parameter = { range?: ParameterRange; options?: ParameterOption[]; maxLength?: number; + /** Physical unit from the model-authored spec, e.g. "mm" or "deg". */ + unit?: string; }; export type Conversation = Omit< diff --git a/src/components/parameter/ParameterInput.tsx b/src/components/parameter/ParameterInput.tsx index ea8ebe17..6795bae0 100644 --- a/src/components/parameter/ParameterInput.tsx +++ b/src/components/parameter/ParameterInput.tsx @@ -10,6 +10,13 @@ import { import { ParameterSlider } from '@/components/parameter/ParameterSlider'; import { Label } from '@/components/ui/label'; import { ColorPicker } from '@/components/parameter/ColorPicker'; +import { + Select, + SelectContent, + SelectItem, + SelectTrigger, + SelectValue, +} from '@/components/ui/select'; export function ParameterInput({ param, @@ -39,6 +46,65 @@ export function ParameterInput({ handleCommit(paramState, value); }; + // Enum-style parameters (model-authored `options` in the parameter spec, + // or Customizer `// [a, b, c]` comments) render as a select — free-text + // input here lets the user type values the OpenSCAD code can't handle. + const scalarOptions = + (!paramState.type || + paramState.type === 'number' || + paramState.type === 'string') && + Array.isArray(paramState.options) && + paramState.options.length > 0 + ? paramState.options + : null; + if (scalarOptions) { + const currentValue = String(paramState.value); + const hasCurrent = scalarOptions.some( + (option) => String(option.value) === currentValue, + ); + return ( +
+ + +
+ ); + } + if (!paramState.type || paramState.type === 'number') { return (
@@ -71,7 +137,8 @@ export function ParameterInput({ }} /> - {isMeasurementParameter(paramState) ? 'mm' : ''} + {paramState.unit ?? + (isMeasurementParameter(paramState) ? 'mm' : '')}
@@ -265,7 +332,8 @@ export function ParameterInput({ }} /> - {isMeasurementParameter(paramState) ? 'mm' : ''} + {paramState.unit ?? + (isMeasurementParameter(paramState) ? 'mm' : '')} diff --git a/src/lib/updateParameter.test.ts b/src/lib/updateParameter.test.ts new file mode 100644 index 00000000..b12aa245 --- /dev/null +++ b/src/lib/updateParameter.test.ts @@ -0,0 +1,62 @@ +import assert from 'node:assert/strict'; +import { describe, it } from 'node:test'; +import { updateParameter } from './utils.ts'; +import type { Parameter } from '@shared/types'; + +function numberParam(name: string, value: number): Parameter { + return { + name, + displayName: name, + value, + defaultValue: value, + type: 'number', + }; +} + +describe('updateParameter', () => { + it('rewrites a scalar assignment in place', () => { + const code = 'cup_height = 100; // [50:5:200]\ncylinder(h=cup_height);'; + assert.equal( + updateParameter(code, numberParam('cup_height', 120)), + 'cup_height = 120; // [50:5:200]\ncylinder(h=cup_height);', + ); + }); + + it('rewrites one component of a vector literal', () => { + // `parseParameters` flattens `size = [30, 20, 10];` into size[0..2] + // sliders — editing size[1] must rewrite the middle element, not + // search for a nonexistent `size[1] = ...;` assignment. + const code = 'size = [30, 20, 10]; // body\ncube(size);'; + assert.equal( + updateParameter(code, numberParam('size[1]', 25)), + 'size = [30, 25, 10]; // body\ncube(size);', + ); + }); + + it('preserves spacing and trailing comments on vector rewrites', () => { + const code = 'size = [ 30 , 20 , 10 ]; // [10:50]'; + assert.equal( + updateParameter(code, numberParam('size[0]', 42)), + 'size = [ 42 , 20 , 10 ]; // [10:50]', + ); + }); + + it('leaves code untouched for an out-of-range vector index', () => { + const code = 'size = [30, 20, 10];'; + assert.equal(updateParameter(code, numberParam('size[5]', 99)), code); + }); + + it('still matches a literal name[i] scalar assignment for strings', () => { + // A string param whose *name* looks indexed must not go down the + // vector path — only number components come from flattening. + const code = 'label = "Cup"; // 24'; + const param: Parameter = { + name: 'label', + displayName: 'Label', + value: 'Mug', + defaultValue: 'Cup', + type: 'string', + }; + assert.equal(updateParameter(code, param), 'label = "Mug"; // 24'); + }); +}); diff --git a/src/lib/utils.ts b/src/lib/utils.ts index ef0dc82d..c9e4c6b8 100644 --- a/src/lib/utils.ts +++ b/src/lib/utils.ts @@ -1,7 +1,9 @@ import { clsx, type ClassValue } from 'clsx'; import { twMerge } from 'tailwind-merge'; -import { Parameter } from '@shared/types'; -import { ModelConfig } from '../types/misc.ts'; +// Type-only imports so `node --test` can run this file without resolving +// the `@shared` path alias (node strips type imports but can't map aliases). +import type { Parameter } from '@shared/types'; +import type { ModelConfig } from '../types/misc.ts'; export function cn(...inputs: ClassValue[]) { return twMerge(clsx(inputs)); @@ -105,6 +107,21 @@ export function validateRedirectUrlServer( } export function updateParameter(code: string, param: Parameter): string { + // `size[1]`-style names come from `parseParameters` flattening a vector + // declaration into per-component sliders. The source still reads + // `size = [30, 20, 10];`, so rewrite the i-th element of the vector + // literal — the scalar regex below would find no `size[1] = ...;` + // assignment and silently no-op the edit. + const vectorComponent = /^(.+)\[(\d+)\]$/.exec(param.name); + if (vectorComponent && (!param.type || param.type === 'number')) { + return updateVectorComponent( + code, + vectorComponent[1], + Number(vectorComponent[2]), + param.value, + ); + } + const escapedName = escapeRegExp(param.name); const regex = new RegExp( `^\\s*(${escapedName}\\s*=\\s*)[^;]+;([\\t\\f\\cK ]*\\/\\/[^\n]*)?`, @@ -147,6 +164,31 @@ export function updateParameter(code: string, param: Parameter): string { } } +function updateVectorComponent( + code: string, + name: string, + index: number, + value: Parameter['value'], +): string { + // Single-line vectors only — `parseParameters` skips multi-line values, + // so a flattened `name[i]` parameter always came from a one-line literal. + const regex = new RegExp( + `^(\\s*${escapeRegExp(name)}\\s*=\\s*\\[)([^\\]\\n]*)(\\][^\\n]*)$`, + 'm', + ); + return code.replace( + regex, + (match, head: string, body: string, tail: string) => { + const items = body.split(','); + if (index < 0 || index >= items.length) return match; + const leading = /^\s*/.exec(items[index])?.[0] ?? ''; + const trailing = /\s*$/.exec(items[index].trimStart())?.[0] ?? ''; + items[index] = `${leading}${value}${trailing}`; + return `${head}${items.join(',')}${tail}`; + }, + ); +} + export function getDiffString(param: Parameter) { let diffString: string = ''; let diffNumber: number = 0; diff --git a/src/server/aiChat.ts b/src/server/aiChat.ts index 4fc2e5d7..c934e9e4 100644 --- a/src/server/aiChat.ts +++ b/src/server/aiChat.ts @@ -99,6 +99,9 @@ The build_parametric_model tool input is the artifact shown to the user: - title: short object name - version: "v1" - code: complete raw OpenSCAD code, no markdown, no code fences +- parameters: the typed schema for every user-editable variable in the code — + this drives the parameter panel (sliders, ranges, units, groups) the user + sees next to the live preview After you call build_parametric_model, the browser compiles the OpenSCAD and returns a multi-view preview sheet covering isometric, front, back, left, @@ -168,18 +171,26 @@ BOSL2 library guidance: Parameters: - Declare every editable parameter as a top-of-file variable. - Use full descriptive snake_case names (e.g. \`wheel_radius\`, \`seat_offset\`) — - never abbreviate to single letters or short tokens (\`w_r\`, \`p_s\`). Names - render directly in the parameter panel, so they must read well to the user. -- Annotate each variable with a trailing OpenSCAD Customizer comment so the - UI can render the right widget: - width = 50; // [10:1:200] ← min:step:max for sliders - height = 25; // [5:50] ← min:max - style = "round"; // [round, square, hex] ← enum options - enabled = true; // ← booleans render as switches - label = "Cup"; // 24 ← maxLength for free-form strings -- Optionally put a "// Description of the parameter" comment on the line - ABOVE the variable so the UI can show a description. -- Group related parameters with /* [Group Name] */ section markers. + never abbreviate to single letters or short tokens (\`w_r\`, \`p_s\`). +- Describe every editable variable in the tool's \`parameters\` array. It is + the source of truth for the parameter panel — one entry per variable: + { "name": "cup_height", "label": "Cup Height", "type": "number", + "min": 50, "max": 200, "step": 5, "unit": "mm", + "group": "Body", "description": "Outer height of the mug" } +- For numbers, always provide min/max/step spanning a sensible design range + around the default, and a unit ("mm", "deg") when the value is physical. +- For enum-style strings, list options with values exactly as they appear in + the code: + "options": [{ "value": "round", "label": "Round" }, { "value": "hex" }] +- min/max must bracket the variable's default value in the code. +- Every entry's \`name\` must match a top-of-file variable verbatim — entries + that don't are ignored. One entry on a vector variable + (e.g. \`size = [30, 20, 10];\`) covers all of its components. +- Also annotate each variable with a trailing OpenSCAD Customizer comment + (\`width = 50; // [10:1:200]\`) and group markers (\`/* [Group Name] */\`) + so downloaded .scad files stay editable in the OpenSCAD desktop + Customizer. The \`parameters\` array is what the app's UI uses; keep the + two consistent. Color: - When the model has distinct parts, wrap each in a color() call with a @@ -238,6 +249,17 @@ module torus(r1, r2) { circle(r=r2); } +and its \`parameters\` should look like: + +[ + { "name": "cup_height", "type": "number", "min": 50, "max": 200, "step": 5, "unit": "mm", "group": "Body", "description": "Outer height of the mug" }, + { "name": "cup_radius", "type": "number", "min": 20, "max": 80, "step": 1, "unit": "mm", "group": "Body" }, + { "name": "handle_radius", "type": "number", "min": 15, "max": 60, "step": 1, "unit": "mm", "group": "Handle" }, + { "name": "handle_thickness", "type": "number", "min": 4, "max": 20, "step": 1, "unit": "mm", "group": "Handle" }, + { "name": "wall_thickness", "type": "number", "min": 2, "max": 6, "step": 0.5, "unit": "mm", "group": "Body", "description": "Must stay printable" }, + { "name": "mug_color", "type": "string", "label": "Mug Color", "group": "Appearance" } +] + # What never to say Do not mention tools, APIs, prompts, or implementation details to the user. diff --git a/src/views/EditorView.tsx b/src/views/EditorView.tsx index bed7cd7d..d1f51767 100644 --- a/src/views/EditorView.tsx +++ b/src/views/EditorView.tsx @@ -22,6 +22,7 @@ import { messageRowToChatMessage, type ChatMessage, } from '@/lib/aiMessages'; +import applyParameterSpecs from '@shared/applyParameterSpecs'; import parseParameters from '@shared/parseParameters'; import { supabase } from '@/lib/supabase'; import { updateParameter } from '@/lib/utils'; @@ -431,17 +432,25 @@ function ConversationEditor() { const handleViewArtifact = useCallback( (artifact: ParametricArtifact, messageId: string) => { baseCodeRef.current = artifact.code; - // Parameters are derived from the OpenSCAD source — same code always - // yields the same ``, no matter which model wrote - // it. The current values come from the (possibly edited) artifact - // code; the `defaultValue` (Reset target / slider home / auto range) - // comes from `metadata.originalCode` — the model's first-authored - // source — so an in-place parameter edit doesn't redefine the default - // on the next reload. Read from the cache to keep this callback stable. + // Parameter existence and current values are derived from the OpenSCAD + // source — same code always yields the same ``, no + // matter which model wrote it. The model-authored `artifact.parameters` + // schema then overlays presentation metadata (ranges, units, labels, + // groups, enum options); entries that don't match a real variable are + // dropped, so the parse validates the schema. The `defaultValue` + // (Reset target / slider home / auto range) comes from + // `metadata.originalCode` — the model's first-authored source — so an + // in-place parameter edit doesn't redefine the default on the next + // reload. Read from the cache to keep this callback stable. const originalCode = queryClient .getQueryData(['messages', conversation.id]) ?.find((row) => row.id === messageId)?.metadata?.originalCode; - setParameters(mergeParameterDefaults(artifact.code, originalCode)); + setParameters( + applyParameterSpecs( + mergeParameterDefaults(artifact.code, originalCode), + artifact.parameters, + ), + ); setCurrentOutput(undefined); setDxfExporter(() => null); setActivePreview({ type: 'artifact', messageId, artifact }); diff --git a/src/views/ShareView.tsx b/src/views/ShareView.tsx index 08a314c7..910aaac9 100644 --- a/src/views/ShareView.tsx +++ b/src/views/ShareView.tsx @@ -9,6 +9,7 @@ import { ConversationContext } from '@/contexts/ConversationContext'; import { messageRowToChatMessage } from '@/lib/aiMessages'; import { supabase } from '@/lib/supabase'; import { updateParameter } from '@/lib/utils'; +import applyParameterSpecs from '@shared/applyParameterSpecs'; import parseParameters from '@shared/parseParameters'; import type { AppUIMessage } from '@shared/chatAi'; import { isParametricArtifact } from '@shared/parametricParts'; @@ -156,7 +157,12 @@ function ConversationShare({ conversation, messages }: ConversationShareProps) { lastAutoAppliedPreviewKeyRef.current = key; if (latest.type === 'artifact') { baseCodeRef.current = latest.artifact.code; - setParameters(parseParameters(latest.artifact.code)); + setParameters( + applyParameterSpecs( + parseParameters(latest.artifact.code), + latest.artifact.parameters, + ), + ); setCurrentOutput(undefined); setActivePreview({ type: 'artifact', @@ -178,7 +184,12 @@ function ConversationShare({ conversation, messages }: ConversationShareProps) { const handleViewArtifact = useCallback( (artifact: ParametricArtifact, messageId: string) => { baseCodeRef.current = artifact.code; - setParameters(parseParameters(artifact.code)); + setParameters( + applyParameterSpecs( + parseParameters(artifact.code), + artifact.parameters, + ), + ); setCurrentOutput(undefined); setActivePreview({ type: 'artifact', messageId, artifact }); setMobilePreviewVersion((version) => version + 1);