fix(service-settings): remove step 0.1 from ai.temperature — the window is the true domain (#6550)

.changeset/ai-temperature-step-removal.md

@@ -0,0 +1,5 @@
+---
+'@objectstack/service-settings': patch
+---
+
+Remove `step: 0.1` from the `ai.temperature` specifier (#6550). Since #6199 a declared `step` binds as a value constraint on both doors, and temperature's true domain is continuous on [0, 2]: the 0.1 grid refused legal values — `PUT /api/settings/ai` with `temperature: 0.15` was rejected and `OS_AI_TEMPERATURE=0.15` was loudly ignored. Both now work; `min: 0` / `max: 2` stay and keep binding (out-of-window values are still refused in the min/max vocabulary). #6199's grid machinery is untouched and still enforces any key that declares `step`.

packages/services/service-settings/src/manifests/ai.manifest.test.ts

@@ -2,6 +2,7 @@
 
 import { describe, it, expect } from 'vitest';
 import { SettingsManifestSchema } from '@objectstack/spec/system';
+import { SettingsService } from '../settings-service.js';
 import { aiSettingsManifest, aiTestActionHandler, aiTestEmbedderActionHandler } from './ai.manifest.js';
 
 describe('aiSettingsManifest', () => {
@@ -91,6 +92,108 @@ describe('aiTestActionHandler', () => {
   });
 });
 
+/**
+ * #6550 — `temperature` declares a window, not a grid. Since #6199 a declared
+ * `step` binds as a value constraint on both doors, and temperature's true
+ * domain is continuous on [0, 2]: the old `step: 0.1` refused legal values
+ * (`0.15`), the same declared-narrower-than-true-domain shape the #5712 ruling
+ * corrected for timezone/currency. A slider's step feel is a UI-layer display
+ * concern, not a contract refusal; `min`/`max` describe the real domain and
+ * stay binding. #6199's grid machinery is untouched — its enforcement and
+ * tests stand for any key that still declares `step`.
+ */
+describe('aiSettingsManifest — temperature declares a window, not a grid (#6550)', () => {
+  const byKey = (k: string) => (aiSettingsManifest.specifiers as any[]).find((s) => s.key === k);
+
+  it('declares min 0 / max 2 and NO step, and that round-trips the spec parse', () => {
+    const t = byKey('temperature');
+    expect(t.min).toBe(0);
+    expect(t.max).toBe(2);
+    expect(t.step).toBeUndefined();
+    const parsed = SettingsManifestSchema.parse(aiSettingsManifest) as any;
+    const pt = parsed.specifiers.find((s: any) => s.key === 'temperature');
+    expect(pt.min).toBe(0);
+    expect(pt.max).toBe(2);
+    expect(pt.step).toBeUndefined();
+  });
+
+  it('write door: accepts 0.15 — the refusal that forced the ruling — and still 0.7', async () => {
+    // `temperature` is `visible: "${data.provider !== 'memory'}"` and the
+    // default provider is `memory`, so the patch carries a real provider (and
+    // its required key) — otherwise the TOUCH/visible contract skips the
+    // specifier entirely and this test would be green for the wrong reason.
+    const svc = new SettingsService({ env: {} });
+    svc.registerManifest(aiSettingsManifest);
+    await expect(
+      svc.setMany('ai', { provider: 'openai', openai_api_key: 'sk-test', temperature: 0.15 }),
+    ).resolves.toBeDefined();
+    expect((await svc.get('ai', 'temperature')).value).toBe(0.15);
+    // …and the values the slider emits keep working, of course.
+    await expect(
+      svc.setMany('ai', { provider: 'openai', openai_api_key: 'sk-test', temperature: 0.7 }),
+    ).resolves.toBeDefined();
+    expect((await svc.get('ai', 'temperature')).value).toBe(0.7);
+  });
+
+  it('write door: the window still binds — out-of-range values are refused in the min/max vocabulary', async () => {
+    const svc = new SettingsService({ env: {} });
+    svc.registerManifest(aiSettingsManifest);
+    await expect(
+      svc.setMany('ai', { provider: 'openai', openai_api_key: 'sk-test', temperature: 2.5 }),
+    ).rejects.toMatchObject({
+      code: 'SETTINGS_VALIDATION',
+      fields: [
+        { field: 'temperature', code: 'max_value', constraint: { min: 0, max: 2 }, value: 2.5 },
+      ],
+    });
+    await expect(
+      svc.setMany('ai', { provider: 'openai', openai_api_key: 'sk-test', temperature: -0.5 }),
+    ).rejects.toMatchObject({
+      code: 'SETTINGS_VALIDATION',
+      fields: [
+        { field: 'temperature', code: 'min_value', constraint: { min: 0, max: 2 }, value: -0.5 },
+      ],
+    });
+    // Atomic: nothing landed.
+    expect((await svc.get('ai', 'temperature')).source).toBe('default');
+  });
+
+  it('env door: OS_AI_TEMPERATURE=0.15 wins the cascade and locks the key', async () => {
+    // The mirror of the write door — #6199 judges both doors at the ONE
+    // decision point, so removing the grid frees this door too.
+    const errors: string[] = [];
+    const svc = new SettingsService({
+      env: { OS_AI_TEMPERATURE: '0.15' },
+      logger: { error: (m: string) => void errors.push(m) },
+    });
+    svc.registerManifest(aiSettingsManifest);
+    const r = await svc.get('ai', 'temperature');
+    expect(r.value).toBe(0.15);
+    expect(r.source).toBe('env');
+    expect(r.locked).toBe(true);
+    expect(r.cascadeChain?.some((e) => e.scope === 'env')).toBe(true);
+    expect(errors).toHaveLength(0);
+  });
+
+  it('env door: an out-of-window OS_AI_TEMPERATURE is still loudly ignored', async () => {
+    // The window survives the grid's removal: `2.5` sits outside `max: 2`.
+    const errors: string[] = [];
+    const svc = new SettingsService({
+      env: { OS_AI_TEMPERATURE: '2.5' },
+      logger: { error: (m: string) => void errors.push(m) },
+    });
+    svc.registerManifest(aiSettingsManifest);
+    const r = await svc.get('ai', 'temperature');
+    expect(r.value).toBe(0.7); // the manifest default, not 2.5
+    expect(r.source).toBe('default');
+    expect(r.locked).toBe(false);
+    expect(errors).toHaveLength(1);
+    expect(errors[0]).toContain('OS_AI_TEMPERATURE');
+    expect(errors[0]).toContain('IGNORED');
+    expect(errors[0]).toContain('does NOT take effect');
+  });
+});
+
 describe('aiSettingsManifest — embedder section', () => {
   it('exposes embedder_provider select with 10 options incl. none + 5 Chinese providers', () => {
     const f = (aiSettingsManifest.specifiers as any[]).find(

packages/services/service-settings/src/manifests/ai.manifest.ts

@@ -185,8 +185,12 @@ const manifest = {
     { type: 'group', id: 'defaults', label: 'Generation defaults', required: false,
       description: 'Applied when an agent or chat request does not specify its own value.',
       visible: "${data.provider !== 'memory'}" },
+    // No `step` on purpose (#6550 ruling): since #6199 a declared `step` binds
+    // as a value constraint on both doors, and temperature's true domain is
+    // continuous on [0, 2] — a 0.1 grid refused legal values like `0.15`.
+    // `min`/`max` stay: the window IS the true domain, so it binds honestly.
     { type: 'slider', key: 'temperature', label: 'Temperature',
-      required: false, default: 0.7, min: 0, max: 2, step: 0.1,
+      required: false, default: 0.7, min: 0, max: 2,
       description: '0 = deterministic, 2 = highly creative.',
       visible: "${data.provider !== 'memory'}" },
     { type: 'number', key: 'max_tokens', label: 'Max output tokens',

packages/services/service-settings/src/settings-service.test.ts

@@ -1388,11 +1388,14 @@ describe('SettingsService — env overrides are checked against declared windows
  * for a renderer that does not exist. A declaration with no consumer at all is
  * the ADR-0049 hole, not a UI affordance.
  *
- * The consequence is real and was accepted at ruling time: `ai.temperature`
- * declares `min: 0, max: 2, step: 0.1`, and `0.15` — a perfectly sensible
- * temperature for the model behind it — is now refused. That is the manifest's
- * declaration binding as written; whether it SHOULD declare a 0.1 grid is the
- * manifest owner's question, not this gate's.
+ * The consequence was real and was accepted at ruling time: `ai.temperature`
+ * then declared `min: 0, max: 2, step: 0.1`, so `0.15` — a perfectly sensible
+ * temperature for the model behind it — was refused. The manifest owner's
+ * question that ruling left open was answered by #6550: the grid came OFF the
+ * ai manifest (temperature's true domain is continuous on [0, 2]), so the
+ * fixtures below are synthetic step-declaring specifiers. The machinery they
+ * pin is unchanged and still binds any key that declares `step`; the real ai
+ * manifest's post-#6550 behaviour is pinned in `manifests/ai.manifest.test.ts`.
  */
 describe('SettingsService — the declared step grid is enforced at save time (#6199)', () => {
   /**
@@ -1626,33 +1629,12 @@ describe('SettingsService — the declared step grid is enforced at save time (#
     expect(err.fields[0].constraint).toMatchObject({ step: 100, min: 0 });
   });
 
-  it('binds the real ai manifest — the consequence accepted at ruling time', async () => {
-    // The repo's ONLY `step` declaration: `ai.temperature`, `min: 0, max: 2,
-    // step: 0.1`. Under enforcement `0.15` is refused, and that is the
-    // declaration binding as written rather than a defect of this gate.
-    // `temperature` is `visible: "${data.provider !== 'memory'}"` and the
-    // default provider is `memory`, so the patch carries a real provider (and
-    // its required key) — otherwise the TOUCH/visible contract skips the
-    // specifier entirely, which would make this test green for the wrong reason.
-    const svc = new SettingsService({ env: {} });
-    svc.registerManifest(aiSettingsManifest);
-
-    await expect(
-      svc.setMany('ai', { provider: 'openai', openai_api_key: 'sk-test', temperature: 0.15 }),
-    ).rejects.toMatchObject({
-      code: 'SETTINGS_VALIDATION',
-      fields: [
-        { field: 'temperature', code: 'invalid_value', constraint: { step: 0.1, min: 0 }, value: 0.15 },
-      ],
-    });
-    // Nothing was stored — the whole batch is atomic.
-    expect((await svc.get('ai', 'provider')).value).toBe('memory');
-    // …and the on-grid values the slider actually emits still go through.
-    await expect(
-      svc.setMany('ai', { provider: 'openai', openai_api_key: 'sk-test', temperature: 0.7 }),
-    ).resolves.toBeDefined();
-    expect((await svc.get('ai', 'temperature')).value).toBe(0.7);
-  });
+  // Until #6550 this block also pinned the one real declaration the gate
+  // bound: `ai.temperature`'s `step: 0.1` refusing `0.15`. That ruling took
+  // the grid off the ai manifest (temperature's true domain is continuous),
+  // so the real-manifest pin MOVED with the declaration — both doors'
+  // post-#6550 behaviour is pinned in `manifests/ai.manifest.test.ts`, and
+  // the grid machinery keeps binding through the synthetic fixtures above.
 });
 
 /**
@@ -1661,27 +1643,45 @@ describe('SettingsService — the declared step grid is enforced at save time (#
  * comparison in two places is how the env half came to disagree with the save
  * half in the first place. `step` rides `DeclaredBounds` and
  * `firstRangeViolation`, so it reaches both doors by construction.
+ *
+ * The fixture is synthetic since #6550 took the grid off `ai.temperature`
+ * (this machinery needs a step-declaring key to bind, and the repo no longer
+ * ships one); it keeps the exact shape `ai.temperature` had at ruling time.
+ * The real ai manifest's env door is pinned in `manifests/ai.manifest.test.ts`.
  */
 describe('SettingsService — env overrides are checked against the declared step grid (#6199)', () => {
+  const gridEnvManifest = {
+    namespace: 'gridenv',
+    version: 1,
+    label: 'Grid env',
+    scope: 'global',
+    readPermission: 'setup.access',
+    writePermission: 'setup.access',
+    specifiers: [
+      { type: 'slider', key: 'temperature', label: 'Temperature', required: false,
+        default: 0.7, min: 0, max: 2, step: 0.1 },
+    ],
+  } as any;
+
   const spyLogger = () => {
     const errors: string[] = [];
     return { errors, logger: { error: (m: string) => void errors.push(m) } };
   };
 
-  it('ignores an off-grid OS_AI_TEMPERATURE and resolves the manifest default instead', async () => {
+  it('ignores an off-grid OS_GRIDENV_TEMPERATURE and resolves the manifest default instead', async () => {
     const { errors, logger } = spyLogger();
-    const svc = new SettingsService({ env: { OS_AI_TEMPERATURE: '0.15' }, logger });
-    svc.registerManifest(aiSettingsManifest);
+    const svc = new SettingsService({ env: { OS_GRIDENV_TEMPERATURE: '0.15' }, logger });
+    svc.registerManifest(gridEnvManifest);
 
-    const r = await svc.get('ai', 'temperature');
+    const r = await svc.get('gridenv', 'temperature');
     expect(r.value).toBe(0.7); // the manifest default, not 0.15
     expect(r.source).toBe('default');
     // Not in force, so it pins nothing either — read and write agree.
     expect(r.locked).toBe(false);
     expect(r.cascadeChain?.some((e) => e.scope === 'env')).toBe(false);
 
     expect(errors).toHaveLength(1);
-    expect(errors[0]).toContain('OS_AI_TEMPERATURE');
+    expect(errors[0]).toContain('OS_GRIDENV_TEMPERATURE');
     // The grid breach gets its OWN sentence, not the window template: the value
     // sits squarely inside `min 0, max 2`, so "is outside the declared step"
     // would be a false description of what happened.
@@ -1696,10 +1696,10 @@ describe('SettingsService — env overrides are checked against the declared ste
     // "env never applies to a stepped key". `1.2` is another float trap:
     // `1.2 / 0.1` is `11.999999999999998`.
     const { errors, logger } = spyLogger();
-    const svc = new SettingsService({ env: { OS_AI_TEMPERATURE: '1.2' }, logger });
-    svc.registerManifest(aiSettingsManifest);
+    const svc = new SettingsService({ env: { OS_GRIDENV_TEMPERATURE: '1.2' }, logger });
+    svc.registerManifest(gridEnvManifest);
 
-    const r = await svc.get('ai', 'temperature');
+    const r = await svc.get('gridenv', 'temperature');
     expect(r.value).toBe(1.2);
     expect(r.source).toBe('env');
     expect(r.locked).toBe(true);
@@ -1708,12 +1708,12 @@ describe('SettingsService — env overrides are checked against the declared ste
 
   it('reports the misconfiguration at registration, and says it ONCE', async () => {
     const { errors, logger } = spyLogger();
-    const svc = new SettingsService({ env: { OS_AI_TEMPERATURE: '0.15' }, logger });
+    const svc = new SettingsService({ env: { OS_GRIDENV_TEMPERATURE: '0.15' }, logger });
     expect(errors).toHaveLength(0);
-    svc.registerManifest(aiSettingsManifest);
+    svc.registerManifest(gridEnvManifest);
     expect(errors).toHaveLength(1);
-    for (let i = 0; i < 5; i++) await svc.get('ai', 'temperature');
-    await svc.getNamespace('ai');
+    for (let i = 0; i < 5; i++) await svc.get('gridenv', 'temperature');
+    await svc.getNamespace('gridenv');
     expect(errors).toHaveLength(1);
   });
 
@@ -1723,16 +1723,14 @@ describe('SettingsService — env overrides are checked against the declared ste
     // nothing (env ignored, UI refused) would be a lockout only an env edit
     // could clear.
     const { logger } = spyLogger();
-    const svc = new SettingsService({ env: { OS_AI_TEMPERATURE: '0.15' }, logger });
-    svc.registerManifest(aiSettingsManifest);
+    const svc = new SettingsService({ env: { OS_GRIDENV_TEMPERATURE: '0.15' }, logger });
+    svc.registerManifest(gridEnvManifest);
 
-    expect((await svc.get('ai', 'temperature')).locked).toBe(false);
-    await expect(
-      svc.setMany('ai', { provider: 'openai', openai_api_key: 'sk-test', temperature: 0.9 }),
-    ).resolves.toBeDefined();
-    const after = await svc.get('ai', 'temperature');
+    expect((await svc.get('gridenv', 'temperature')).locked).toBe(false);
+    await expect(svc.setMany('gridenv', { temperature: 0.9 })).resolves.toBeDefined();
+    const after = await svc.get('gridenv', 'temperature');
     expect(after.value).toBe(0.9);
-    expect(after.source).toBe('global'); // the ai manifest is `scope: 'global'`
+    expect(after.source).toBe('global'); // the fixture is `scope: 'global'`
   });
 });