From 963c645b0e3249ebf7037ec17336da5eab655a32 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sat, 8 Aug 2026 09:17:53 -0700 Subject: [PATCH 1/5] Add bench-wide chat defaults with per-channel context-window overrides Channels can now inherit their context window from a new per-tenant chat_bench_settings table instead of only ever reading a code-level constant; chat/contextWindow becomes nullable (null/absent means inherit) and a new resolveContextWindow function folds a channel's override against the bench default into one effective value, exposed through GET/PATCH /channels/:id/settings and a new GET/PATCH /bench/settings route. A migration makes every pre-existing row's implicit default explicit as null rather than silently reinterpreting already-set values. --- packages/chat/src/channel-service.ts | 17 +- packages/chat/src/channel-settings.ts | 146 +++++++++++++-- packages/chat/src/migrations.ts | 34 ++++ packages/chat/src/routes.ts | 71 +++++++- packages/chat/src/schema.ts | 17 ++ packages/chat/src/store.ts | 68 ++++++- packages/chat/test/channel-settings.test.ts | 185 ++++++++++++++++++++ packages/chat/test/migrations.test.ts | 79 +++++++-- packages/chat/test/store.test.ts | 24 +++ 9 files changed, 594 insertions(+), 47 deletions(-) diff --git a/packages/chat/src/channel-service.ts b/packages/chat/src/channel-service.ts index bf6b787c8..483c1857e 100644 --- a/packages/chat/src/channel-service.ts +++ b/packages/chat/src/channel-service.ts @@ -20,10 +20,10 @@ import { type ParticipantRecord, } from "./participants"; import { - contextWindowOf, - DEFAULT_CONTEXT_WINDOW, + benchContextWindowOf, kindOf, participantsOf, + resolveContextWindow, } from "./channel-settings"; import type { ChannelLauncher, @@ -300,7 +300,7 @@ async function loadChannelContext(input: { } export type SendChannelMessageDeps = { - readonly store: Pick; + readonly store: Pick; readonly platform: Pick; }; @@ -363,10 +363,13 @@ export async function sendChannelMessage( channelId: input.channelId, excludeMailId: sent.id, participants, - contextWindow: - settingsRow !== undefined - ? contextWindowOf(settingsRow.settings) - : DEFAULT_CONTEXT_WINDOW, + contextWindow: resolveContextWindow( + settingsRow?.settings ?? {}, + benchContextWindowOf( + (await deps.store.getBenchSettings(input.tenantId))?.settings ?? + {}, + ), + ).value, }) : undefined; const fanoutParts = diff --git a/packages/chat/src/channel-settings.ts b/packages/chat/src/channel-settings.ts index 85c3e9de7..003003587 100644 --- a/packages/chat/src/channel-settings.ts +++ b/packages/chat/src/channel-settings.ts @@ -16,24 +16,37 @@ import { const PatchSettingsBody = type("Record"); +// `chat/contextWindow` is nullable: `null` (or the key's absence) means +// "inherit the bench-wide default", a number is an explicit per-channel +// override. This is the Discord "use server default" shape — see +// `resolveContextWindow` below for how the two are told apart and folded +// into one effective value. export const ChatNamespaceSchemas: Readonly>> = { "chat/kind": type("string"), "chat/name": type("string"), "chat/pinned": type("boolean"), "chat/participants": ParticipantsSetting, + "chat/contextWindow": type("number | null"), +}; + +// The bench-wide chat defaults vocabulary: currently just the default +// context window every channel inherits unless it sets its own override. +// Kept as its own schema table (rather than folded into +// `ChatNamespaceSchemas`) because a bench default is never nullable — there +// is nothing beneath it to inherit from. +export const ChatBenchNamespaceSchemas: Readonly< + Record> +> = { "chat/contextWindow": type("number"), }; export class SettingsValidationError extends Error {} -/** - * Validates a settings PATCH payload: `chat/*` keys are checked - * against the package's own strict schema per key, while any other - * `/*` namespace passes through opaquely. That asymmetry is the - * extension contract, not a fallback — a foreign package's settings - * are simply not this package's to validate. - */ -export function validateSettingsPatch(body: unknown): Record { +function validatePatchAgainst( + body: unknown, + schemas: Readonly>>, + namespace: string, +): Record { const parsed = PatchSettingsBody(body); if (parsed instanceof type.errors) { throw new SettingsValidationError( @@ -42,8 +55,8 @@ export function validateSettingsPatch(body: unknown): Record { } const validated: Record = {}; for (const [key, value] of Object.entries(parsed)) { - if (key.startsWith("chat/")) { - const schema = ChatNamespaceSchemas[key]; + if (key.startsWith(namespace)) { + const schema = schemas[key]; if (schema === undefined) { throw new SettingsValidationError(`unknown chat setting "${key}"`); } @@ -61,6 +74,30 @@ export function validateSettingsPatch(body: unknown): Record { return validated; } +/** + * Validates a settings PATCH payload: `chat/*` keys are checked + * against the package's own strict schema per key, while any other + * `/*` namespace passes through opaquely. That asymmetry is the + * extension contract, not a fallback — a foreign package's settings + * are simply not this package's to validate. + */ +export function validateSettingsPatch(body: unknown): Record { + return validatePatchAgainst(body, ChatNamespaceSchemas, "chat/"); +} + +/** + * Validates a bench-wide settings PATCH payload the same way + * `validateSettingsPatch` validates a channel's, against the bench + * defaults vocabulary instead. A bench default carries no inherit case of + * its own, so every `chat/*` key here is required to be its real type, + * never `null`. + */ +export function validateBenchSettingsPatch( + body: unknown, +): Record { + return validatePatchAgainst(body, ChatBenchNamespaceSchemas, "chat/"); +} + /** * A channel's kind, read off its settings — the same "settings is the * source of truth" surface `participantsOf` reads. Defaults to @@ -80,21 +117,94 @@ export const DEFAULT_CONTEXT_WINDOW = 20; * value can never turn a mention fan-out into a token bomb. */ export const MAX_CONTEXT_WINDOW = 200; +function clampWindow(raw: unknown): number | undefined { + if (typeof raw !== "number" || !Number.isInteger(raw) || raw < 0) { + return undefined; + } + return Math.min(raw, MAX_CONTEXT_WINDOW); +} + /** * A channel's context-window size, read off its settings the same way * `kindOf` reads kind: a non-negative integer, where `0` disables the * channel-context block entirely. Absent or invalid values (wrong type, - * negative, non-integer) fall back to `DEFAULT_CONTEXT_WINDOW` rather - * than trusting the jsonb shape; anything above `MAX_CONTEXT_WINDOW` is - * clamped down to it — validation at the trust boundary, not a - * fallback path. + * negative, non-integer, `null`) fall back to `DEFAULT_CONTEXT_WINDOW` + * rather than trusting the jsonb shape; anything above + * `MAX_CONTEXT_WINDOW` is clamped down to it — validation at the trust + * boundary, not a fallback path. + * + * This reads the code-level default directly, with no notion of a + * bench-wide override — callers that have a bench default in hand should + * use `resolveContextWindow` instead, which is the inherit/override-aware + * successor to this function. */ export function contextWindowOf(settings: Record): number { - const raw = settings["chat/contextWindow"]; - if (typeof raw !== "number" || !Number.isInteger(raw) || raw < 0) { - return DEFAULT_CONTEXT_WINDOW; + return clampWindow(settings["chat/contextWindow"]) ?? DEFAULT_CONTEXT_WINDOW; +} + +/** + * A bench's default context window, read off its bench-wide settings the + * same way `contextWindowOf` reads a channel's: absent or invalid falls + * back to `DEFAULT_CONTEXT_WINDOW`, and anything oversized clamps to + * `MAX_CONTEXT_WINDOW`. A bench default is never itself "inherited" — + * there is nothing beneath it — so this never returns a null/override + * distinction, only a plain number. + */ +export function benchContextWindowOf( + settings: Record, +): number { + return clampWindow(settings["chat/contextWindow"]) ?? DEFAULT_CONTEXT_WINDOW; +} + +export type ContextWindowSource = "inherit" | "override"; + +export interface ResolvedContextWindow { + readonly value: number; + readonly source: ContextWindowSource; +} + +const BenchDefaultInput = type("number.integer >= 0"); + +/** + * Folds a channel's `chat/contextWindow` override against its bench's + * default into the one effective value a message send actually uses — + * the "Use bench default" vs "Override" distinction the channel settings + * panel renders as a two-state control. + * + * `null` or an absent key on the channel means inherit: the resolved + * value is the bench default, clamped the same way a channel override + * would be. Any other valid number is an explicit override, clamped to + * `MAX_CONTEXT_WINDOW` on its own. An invalid override (wrong type, + * negative, non-integer) is treated the same as absent — it inherits, + * rather than silently coercing to some other number. + * + * `benchDefault` is trusted to already be a valid, clamped context + * window (as `benchContextWindowOf` produces) — this throws loudly + * rather than accepting a malformed bench default, since a bad bench + * default would otherwise silently corrupt every inheriting channel's + * effective value. + */ +export function resolveContextWindow( + channelSettings: Record, + benchDefault: number, +): ResolvedContextWindow { + const validatedDefault = BenchDefaultInput(benchDefault); + if (validatedDefault instanceof type.errors) { + throw new Error( + `resolveContextWindow: invalid bench default: ${validatedDefault.summary}`, + ); } - return Math.min(raw, MAX_CONTEXT_WINDOW); + const clampedDefault = Math.min(validatedDefault, MAX_CONTEXT_WINDOW); + + const raw = channelSettings["chat/contextWindow"]; + if (raw === undefined || raw === null) { + return { value: clampedDefault, source: "inherit" }; + } + const override = clampWindow(raw); + if (override === undefined) { + return { value: clampedDefault, source: "inherit" }; + } + return { value: override, source: "override" }; } export function participantsOf( diff --git a/packages/chat/src/migrations.ts b/packages/chat/src/migrations.ts index 1fecdecc1..7e190ac0a 100644 --- a/packages/chat/src/migrations.ts +++ b/packages/chat/src/migrations.ts @@ -86,6 +86,40 @@ export const chatMigrations: readonly ChatMigration[] = [ ON "channel_tenancy" ("parent_tenant_id"); `, }, + { + name: "0007_chat_bench_settings", + sql: ` + CREATE TABLE IF NOT EXISTS "chat_bench_settings" ( + "tenant_id" text NOT NULL, + "settings" jsonb NOT NULL, + "updated_by" text NOT NULL, + "updated_at" timestamptz NOT NULL DEFAULT now(), + PRIMARY KEY ("tenant_id") + ); + `, + }, + // Before this rollout, `channel_settings.settings->>'chat/contextWindow'` + // had exactly one meaning: an explicit per-channel value, read back by + // `contextWindowOf` with a code-level fallback of 20 for any row that + // never set the key at all. There was no bench-wide default for a + // channel to "inherit" — every existing value already on a row is + // therefore a real, deliberately-set override, not some default that + // happened to get written. Introducing bench defaults must not + // silently reinterpret those rows as "inheriting" (they were never + // inheriting anything), so this migration leaves every row that + // already carries the key untouched — it stays an explicit override — + // and only touches rows with no key at all, making that absence + // explicit as `null` (inherit) rather than continuing to rely on an + // implicit, code-only fallback now that a real bench-wide default + // exists to inherit from. + { + name: "0008_channel_context_window_explicit_inherit", + sql: ` + UPDATE "channel_settings" + SET "settings" = jsonb_set("settings", '{chat/contextWindow}', 'null'::jsonb) + WHERE NOT ("settings" ? 'chat/contextWindow'); + `, + }, ]; // Bookkeeping table for this package's own migrations. Named diff --git a/packages/chat/src/routes.ts b/packages/chat/src/routes.ts index 3da7f0ecf..aa1cf1ddb 100644 --- a/packages/chat/src/routes.ts +++ b/packages/chat/src/routes.ts @@ -40,10 +40,13 @@ import { type ChannelParticipantState, } from "./settings-control"; import { + benchContextWindowOf, channelView, kindOf, participantsOf, + resolveContextWindow, SettingsValidationError, + validateBenchSettingsPatch, validateSettingsPatch, } from "./channel-settings"; import { launchAndJoinAgent, sendChannelMessage } from "./channel-service"; @@ -711,6 +714,70 @@ export function createChatRoutes(deps: CreateChatRoutesDeps): Hono { }, ); + async function withResolvedContextWindow( + tenantId: string, + row: { channelId: string; settings: Record }, + ) { + const bench = await deps.store.getBenchSettings(tenantId); + const resolved = resolveContextWindow( + row.settings, + benchContextWindowOf(bench?.settings ?? {}), + ); + return { + ...channelView(row), + settings: row.settings, + contextWindow: resolved, + }; + } + + app.get( + "/bench/settings", + deps.requireGrant("workflow-run:*", "read"), + async (c) => { + const tenant = c.get("tenant"); + const row = await deps.store.getBenchSettings(tenant.id); + const settings = row?.settings ?? {}; + return c.json({ + settings, + contextWindow: benchContextWindowOf(settings), + }); + }, + ); + + app.patch( + "/bench/settings", + deps.requireGrant("workflow-run:*", "write"), + async (c) => { + const tenant = c.get("tenant"); + const principal = c.get("principal"); + + let patch: Record; + try { + patch = validateBenchSettingsPatch( + await c.req.json().catch(() => undefined), + ); + } catch (err) { + if (err instanceof SettingsValidationError) { + return c.json(ErrorEnvelope("bad_request", err.message), 400); + } + throw err; + } + + const existing = await deps.store.getBenchSettings(tenant.id); + const merged = { ...(existing?.settings ?? {}), ...patch }; + const row = await deps.store.upsertBenchSettings({ + tenantId: tenant.id, + settings: merged, + updatedBy: principal.id, + }); + + return c.json({ + settings: row.settings, + contextWindow: benchContextWindowOf(row.settings), + }); + }, + ); + app.get( "/channels/:id/settings", deps.requireGrant(idResource("workflow-run", "id"), "read"), @@ -721,7 +788,7 @@ export function createChatRoutes(deps: CreateChatRoutesDeps): Hono { if (row === undefined) { return c.json(ErrorEnvelope("not_found", "channel not found"), 404); } - return c.json({ ...channelView(row), settings: row.settings }); + return c.json(await withResolvedContextWindow(tenant.id, row)); }, ); @@ -819,7 +886,7 @@ export function createChatRoutes(deps: CreateChatRoutesDeps): Hono { data: { updatedBy: principal.id, settings: row.settings }, }); - return c.json({ ...channelView(row), settings: row.settings }); + return c.json(await withResolvedContextWindow(tenant.id, row)); }, ); diff --git a/packages/chat/src/schema.ts b/packages/chat/src/schema.ts index e8092b45d..4256545e7 100644 --- a/packages/chat/src/schema.ts +++ b/packages/chat/src/schema.ts @@ -32,6 +32,23 @@ export const channelSettings = pgTable( (table) => [primaryKey({ columns: [table.tenantId, table.channelId] })], ); +/** + * Bench-wide chat defaults — one row per tenant, the same + * record-as-truth jsonb shape as `channelSettings` (a `"chat/..."` + * namespaced blob rather than a column per setting). A channel with no + * override for a given key inherits its value from here; see + * `resolveContextWindow` in `./channel-settings.ts` for how the two are + * folded into one effective value. + */ +export const chatBenchSettings = pgTable("chat_bench_settings", { + tenantId: text("tenant_id").primaryKey(), + settings: jsonb("settings").notNull(), + updatedBy: text("updated_by").notNull(), + updatedAt: timestamp("updated_at", { withTimezone: true }) + .notNull() + .defaultNow(), +}); + /** * Per-principal read cursor for a channel — humans and agents alike, * since both are principals on the platform. `channelId` is the diff --git a/packages/chat/src/store.ts b/packages/chat/src/store.ts index 820fc6642..b5c6ce121 100644 --- a/packages/chat/src/store.ts +++ b/packages/chat/src/store.ts @@ -13,7 +13,7 @@ import { and, eq } from "drizzle-orm"; import type { PostgresJsDatabase } from "drizzle-orm/postgres-js"; -import { channelReadState, channelSettings } from "./schema"; +import { channelReadState, channelSettings, chatBenchSettings } from "./schema"; /** * The drizzle handle `createDrizzleChatStore` operates against. Generic over @@ -48,6 +48,19 @@ export interface UpdateChannelSettingsInput { readonly updatedBy: string; } +export interface ChatBenchSettingsRow { + readonly tenantId: string; + readonly settings: Record; + readonly updatedBy: string; + readonly updatedAt: Date; +} + +export interface UpsertBenchSettingsInput { + readonly tenantId: string; + readonly settings: Record; + readonly updatedBy: string; +} + export interface ReadStateRow { readonly tenantId: string; readonly channelId: string; @@ -79,6 +92,10 @@ export interface ChatStore { updateChannelSettings( input: UpdateChannelSettingsInput, ): Promise; + getBenchSettings(tenantId: string): Promise; + upsertBenchSettings( + input: UpsertBenchSettingsInput, + ): Promise; getReadState( tenantId: string, channelId: string, @@ -160,6 +177,39 @@ export function createDrizzleChatStore>( return row as ChannelSettingsRow; }, + async getBenchSettings(tenantId) { + const [selected] = await db + .select() + .from(chatBenchSettings) + .where(eq(chatBenchSettings.tenantId, tenantId)) + .limit(1); + return selected as ChatBenchSettingsRow | undefined; + }, + + async upsertBenchSettings(input) { + const [row] = await db + .insert(chatBenchSettings) + .values({ + tenantId: input.tenantId, + settings: input.settings, + updatedBy: input.updatedBy, + updatedAt: new Date(), + }) + .onConflictDoUpdate({ + target: chatBenchSettings.tenantId, + set: { + settings: input.settings, + updatedBy: input.updatedBy, + updatedAt: new Date(), + }, + }) + .returning(); + if (row === undefined) { + throw new Error("upsertBenchSettings: upsert returned no row"); + } + return row as ChatBenchSettingsRow; + }, + async getReadState(tenantId, channelId, principalId) { const [row] = await db .select() @@ -208,6 +258,7 @@ export function createDrizzleChatStore>( export function createInMemoryChatStore(): ChatStore { const settingsByKey = new Map(); const readStateByKey = new Map(); + const benchSettingsByTenant = new Map(); const settingsKey = (tenantId: string, channelId: string) => `${tenantId}:${channelId}`; @@ -260,6 +311,21 @@ export function createInMemoryChatStore(): ChatStore { return row; }, + async getBenchSettings(tenantId) { + return benchSettingsByTenant.get(tenantId); + }, + + async upsertBenchSettings(input) { + const row: ChatBenchSettingsRow = { + tenantId: input.tenantId, + settings: input.settings, + updatedBy: input.updatedBy, + updatedAt: new Date(), + }; + benchSettingsByTenant.set(input.tenantId, row); + return row; + }, + async getReadState(tenantId, channelId, principalId) { return readStateByKey.get(readStateKey(tenantId, channelId, principalId)); }, diff --git a/packages/chat/test/channel-settings.test.ts b/packages/chat/test/channel-settings.test.ts index b85802dab..96cfe0448 100644 --- a/packages/chat/test/channel-settings.test.ts +++ b/packages/chat/test/channel-settings.test.ts @@ -5,6 +5,10 @@ import { describe, expect, test } from "bun:test"; import { createChatRoutes } from "../src/routes"; import { decodeParts } from "../src/codec"; +import { + benchContextWindowOf, + resolveContextWindow, +} from "../src/channel-settings"; import { buildDeps, createChannel, @@ -174,6 +178,73 @@ describe("chat/contextWindow", () => { }); }); +describe("resolveContextWindow", () => { + test("an absent chat/contextWindow inherits the bench default", () => { + expect(resolveContextWindow({}, 30)).toEqual({ + value: 30, + source: "inherit", + }); + }); + + test("a null chat/contextWindow inherits the bench default", () => { + expect(resolveContextWindow({ "chat/contextWindow": null }, 30)).toEqual({ + value: 30, + source: "inherit", + }); + }); + + test("an explicit number overrides the bench default", () => { + expect(resolveContextWindow({ "chat/contextWindow": 5 }, 30)).toEqual({ + value: 5, + source: "override", + }); + }); + + test("an invalid override (negative or non-numeric) inherits rather than corrupting the effective value", () => { + expect(resolveContextWindow({ "chat/contextWindow": -3 }, 30)).toEqual({ + value: 30, + source: "inherit", + }); + expect(resolveContextWindow({ "chat/contextWindow": "lots" }, 30)).toEqual({ + value: 30, + source: "inherit", + }); + }); + + test("an oversized override clamps to the maximum, independent of the bench default", () => { + expect(resolveContextWindow({ "chat/contextWindow": 10_000 }, 30)).toEqual({ + value: 200, + source: "override", + }); + }); + + test("an oversized bench default clamps too, when inherited", () => { + expect(resolveContextWindow({}, 10_000)).toEqual({ + value: 200, + source: "inherit", + }); + }); + + test("throws loudly on an invalid bench default rather than silently coercing it", () => { + expect(() => resolveContextWindow({}, -1)).toThrow(); + expect(() => resolveContextWindow({}, 1.5)).toThrow(); + }); +}); + +describe("benchContextWindowOf", () => { + test("defaults to 20 when the bench has set nothing", () => { + expect(benchContextWindowOf({})).toBe(20); + }); + + test("reads a bench's own set default", () => { + expect(benchContextWindowOf({ "chat/contextWindow": 50 })).toBe(50); + }); + + test("falls back to the default on an invalid value", () => { + expect(benchContextWindowOf({ "chat/contextWindow": -5 })).toBe(20); + }); +}); + describe("PATCH /channels/:id/settings", () => { test("validates chat/* strictly, passes foreign namespaces opaquely, and sends control mail", async () => { const deps = buildDeps(); @@ -228,4 +299,118 @@ describe("PATCH /channels/:id/settings", () => { expect(response.status).toBe(400); }); + + test("a GET/PATCH response reports the effective contextWindow as inherited by default", async () => { + const deps = buildDeps(); + const app = mountAs(createChatRoutes(deps), "prn_alice"); + const { body: channel } = await createChannel(app, { kind: "channel" }); + + const response = await app.request(`/channels/${channel.id}/settings`); + const body = (await response.json()) as { + contextWindow: { value: number; source: string }; + }; + expect(body.contextWindow).toEqual({ value: 20, source: "inherit" }); + }); + + test("PATCHing an explicit chat/contextWindow reports it as overridden", async () => { + const deps = buildDeps(); + const app = mountAs(createChatRoutes(deps), "prn_alice"); + const { body: channel } = await createChannel(app, { kind: "channel" }); + + const response = await app.request(`/channels/${channel.id}/settings`, { + method: "PATCH", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ "chat/contextWindow": 7 }), + }); + const body = (await response.json()) as { + contextWindow: { value: number; source: string }; + }; + expect(body.contextWindow).toEqual({ value: 7, source: "override" }); + }); + + test("PATCHing chat/contextWindow to null clears an override back to inherit, distinct from omitting the key", async () => { + const deps = buildDeps(); + const app = mountAs(createChatRoutes(deps), "prn_alice"); + const { body: channel } = await createChannel(app, { kind: "channel" }); + + await app.request(`/channels/${channel.id}/settings`, { + method: "PATCH", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ "chat/contextWindow": 7 }), + }); + + const omittedResponse = await app.request( + `/channels/${channel.id}/settings`, + { + method: "PATCH", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ "chat/pinned": true }), + }, + ); + const omittedBody = (await omittedResponse.json()) as { + contextWindow: { value: number; source: string }; + }; + expect(omittedBody.contextWindow).toEqual({ value: 7, source: "override" }); + + const clearedResponse = await app.request( + `/channels/${channel.id}/settings`, + { + method: "PATCH", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ "chat/contextWindow": null }), + }, + ); + const clearedBody = (await clearedResponse.json()) as { + contextWindow: { value: number; source: string }; + settings: Record; + }; + expect(clearedBody.contextWindow).toEqual({ value: 20, source: "inherit" }); + expect(clearedBody.settings["chat/contextWindow"]).toBeNull(); + }); +}); + +describe("GET/PATCH /bench/settings", () => { + test("defaults to the code-level default when the bench has set nothing", async () => { + const deps = buildDeps(); + const app = mountAs(createChatRoutes(deps), "prn_alice"); + + const response = await app.request("/bench/settings"); + const body = (await response.json()) as { contextWindow: number }; + expect(body.contextWindow).toBe(20); + }); + + test("PATCH sets the bench default, which every inheriting channel then reflects", async () => { + const deps = buildDeps(); + const app = mountAs(createChatRoutes(deps), "prn_alice"); + + const patchResponse = await app.request("/bench/settings", { + method: "PATCH", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ "chat/contextWindow": 40 }), + }); + expect(patchResponse.status).toBe(200); + const patchBody = (await patchResponse.json()) as { contextWindow: number }; + expect(patchBody.contextWindow).toBe(40); + + const { body: channel } = await createChannel(app, { kind: "channel" }); + const channelResponse = await app.request( + `/channels/${channel.id}/settings`, + ); + const channelBody = (await channelResponse.json()) as { + contextWindow: { value: number; source: string }; + }; + expect(channelBody.contextWindow).toEqual({ value: 40, source: "inherit" }); + }); + + test("rejects a null bench default — there is nothing beneath it to inherit from", async () => { + const deps = buildDeps(); + const app = mountAs(createChatRoutes(deps), "prn_alice"); + + const response = await app.request("/bench/settings", { + method: "PATCH", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ "chat/contextWindow": null }), + }); + expect(response.status).toBe(400); + }); }); diff --git a/packages/chat/test/migrations.test.ts b/packages/chat/test/migrations.test.ts index 2e0ee8010..bb2514120 100644 --- a/packages/chat/test/migrations.test.ts +++ b/packages/chat/test/migrations.test.ts @@ -8,7 +8,7 @@ import { afterAll, beforeAll, describe, expect, test } from "bun:test"; import postgres from "postgres"; import { e2eDatabaseUrl } from "../../../scripts/e2e/harness"; -import { applyChatMigrations } from "../src/migrations"; +import { applyChatMigrations, chatMigrations } from "../src/migrations"; function scratchUrlFor(e2eUrl: string): string { const url = new URL(e2eUrl); @@ -56,36 +56,34 @@ describeIfDb("applyChatMigrations", () => { } }); - test("applies both tables and is idempotent on a second run", async () => { + const migrationNames = [ + "0001_channel_settings", + "0002_channel_read_state", + "0003_channel_launch", + "0004_channel_launch_noop_inference", + "0005_channel_tenancy", + "0006_channel_tenancy_parent_index", + "0007_chat_bench_settings", + "0008_channel_context_window_explicit_inherit", + ]; + + test("applies every table and is idempotent on a second run", async () => { const first = await applyChatMigrations(scratchUrl); - expect(first.applied).toEqual([ - "0001_channel_settings", - "0002_channel_read_state", - "0003_channel_launch", - "0004_channel_launch_noop_inference", - "0005_channel_tenancy", - "0006_channel_tenancy_parent_index", - ]); + expect(first.applied).toEqual(migrationNames); const second = await applyChatMigrations(scratchUrl); expect(second.applied).toEqual([]); - expect(second.alreadyApplied.sort()).toEqual([ - "0001_channel_settings", - "0002_channel_read_state", - "0003_channel_launch", - "0004_channel_launch_noop_inference", - "0005_channel_tenancy", - "0006_channel_tenancy_parent_index", - ]); + expect(second.alreadyApplied.sort()).toEqual([...migrationNames].sort()); const sql = postgres(scratchUrl, { max: 1, onnotice: () => undefined }); try { const tables = await sql.unsafe( `SELECT table_name FROM information_schema.tables ` + `WHERE table_schema = 'public' AND table_name IN ` + - `('channel_settings', 'channel_read_state', 'channel_launch', 'channel_tenancy')`, + `('channel_settings', 'channel_read_state', 'channel_launch', 'channel_tenancy', 'chat_bench_settings')`, ); expect(tables.map((row) => String(row["table_name"])).sort()).toEqual([ + "chat_bench_settings", "channel_launch", "channel_read_state", "channel_settings", @@ -105,4 +103,47 @@ describeIfDb("applyChatMigrations", () => { await sql.end(); } }); + + test("0008 makes a pre-existing row's absent contextWindow an explicit inherit, leaving a set value untouched", async () => { + const sql = postgres(scratchUrl, { max: 1, onnotice: () => undefined }); + try { + await sql.unsafe(`DELETE FROM "channel_settings"`); + await sql.unsafe( + `INSERT INTO "channel_settings" (tenant_id, channel_id, settings, updated_by) VALUES + ('tnt_1', 'chn_absent', '{"chat/kind": "channel"}'::jsonb, 'prn_1'), + ('tnt_1', 'chn_override', '{"chat/kind": "channel", "chat/contextWindow": 5}'::jsonb, 'prn_1')`, + ); + + // Re-runs 0008's own SQL directly (rather than `applyChatMigrations`, + // whose ledger already marked 0008 applied by the earlier test) to + // exercise its behavior against rows inserted after that first run. + const migration = chatMigrations.find( + (candidate) => + candidate.name === "0008_channel_context_window_explicit_inherit", + ); + if (migration === undefined) { + throw new Error("0008 migration missing from chatMigrations"); + } + await sql.unsafe(migration.sql); + + const rows = await sql.unsafe( + `SELECT channel_id, settings FROM "channel_settings" ORDER BY channel_id`, + ); + const byId = new Map( + rows.map((row) => [String(row["channel_id"]), row["settings"]]), + ); + expect( + (byId.get("chn_absent") as Record)[ + "chat/contextWindow" + ], + ).toBeNull(); + expect( + (byId.get("chn_override") as Record)[ + "chat/contextWindow" + ], + ).toBe(5); + } finally { + await sql.end(); + } + }); }); diff --git a/packages/chat/test/store.test.ts b/packages/chat/test/store.test.ts index 146ad7a3d..6c7324a8a 100644 --- a/packages/chat/test/store.test.ts +++ b/packages/chat/test/store.test.ts @@ -75,6 +75,30 @@ test("updateChannelSettings replaces the settings blob and rejects a missing cha ).rejects.toThrow(); }); +test("getBenchSettings is undefined until a bench sets defaults, then upsertBenchSettings replaces them", async () => { + const store = createInMemoryChatStore(); + expect(await store.getBenchSettings("tnt_1")).toBeUndefined(); + + await store.upsertBenchSettings({ + tenantId: "tnt_1", + settings: { "chat/contextWindow": 30 }, + updatedBy: "prn_1", + }); + const first = await store.getBenchSettings("tnt_1"); + expect(first?.settings["chat/contextWindow"]).toBe(30); + + await store.upsertBenchSettings({ + tenantId: "tnt_1", + settings: { "chat/contextWindow": 45 }, + updatedBy: "prn_2", + }); + const second = await store.getBenchSettings("tnt_1"); + expect(second?.settings["chat/contextWindow"]).toBe(45); + expect(second?.updatedBy).toBe("prn_2"); + + expect(await store.getBenchSettings("tnt_2")).toBeUndefined(); +}); + test("putReadState upserts a per-principal cursor without disturbing other principals", async () => { const store = createInMemoryChatStore(); await store.putReadState({ From c12fb9750cc6b956d8c3231a9e65c25fc1addfc9 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sat, 8 Aug 2026 17:30:53 -0700 Subject: [PATCH 2/5] Refactor settings-ui chat section for bench-wide context-window default MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit chat/contextWindow is now a bench-wide default every channel inherits, so the chat settings section edits only that single number rather than per-channel values. context-window.ts keeps null meaning "not ready to submit" for the bench-default field (a bench default is never itself an inherited value — there is nothing beneath it), distinct from a channel's own nullable override which means "inherit." --- packages/settings-ui/src/chat-section.tsx | 305 ++++-------------- packages/settings-ui/src/context-window.ts | 38 ++- packages/settings-ui/src/index.ts | 2 +- packages/settings-ui/src/strings.ts | 23 +- .../settings-ui/test/context-window.test.ts | 17 +- .../settings-ui/test/raw-id-sweep.test.tsx | 30 +- 6 files changed, 99 insertions(+), 316 deletions(-) diff --git a/packages/settings-ui/src/chat-section.tsx b/packages/settings-ui/src/chat-section.tsx index 57069230f..626098b75 100644 --- a/packages/settings-ui/src/chat-section.tsx +++ b/packages/settings-ui/src/chat-section.tsx @@ -1,22 +1,14 @@ -// The "Chats & channels" settings section: pick a channel, then edit its -// name, pinned flag, and conversation-memory window. Every fetch and mutation -// goes through `@corbits/chat-ui`'s own API client — this section only -// composes the picker and the form around it, never re-implements the wire -// contract. - -import type { Channel, ChannelSettings } from "@corbits/chat-ui"; -import { - getChannelSettings, - listChannels, - patchChannelSettings, -} from "@corbits/chat-ui"; -import { - EmptyState, - Input, - SettingsPanel, - Skeleton, - Switch, -} from "@corbits/react-ui"; +// The "Chats & channels" settings section: the bench-wide chat defaults +// every channel inherits unless it sets its own override. A channel's own +// override now lives on the channel itself (its header's settings panel in +// `@corbits/chat-ui`, not here) — this section is bench-wide defaults only. +// Every fetch and mutation goes through `@corbits/chat-ui`'s own API +// client — this section only composes the form around it, never +// re-implements the wire contract. + +import { getBenchChatSettings, patchBenchChatSettings } from "@corbits/chat-ui"; +import type { BenchChatSettings } from "@corbits/chat-ui"; +import { EmptyState, Input, SettingsPanel, Skeleton } from "@corbits/react-ui"; import { CircleAlert } from "lucide-react"; import { useEffect, useState } from "react"; @@ -24,183 +16,28 @@ import { contextWindowLabel, parseContextWindowInput } from "./context-window"; import { errorMessage, type LoadState } from "./load-state"; import { SETTINGS_STRINGS } from "./strings"; -function rawContextWindow(settings: ChannelSettings): number | undefined { - const value = settings.settings["chat/contextWindow"]; - return typeof value === "number" ? value : undefined; -} - -export function ChannelPicker({ - channels, - selectedId, - onSelect, -}: { - readonly channels: readonly Channel[]; - readonly selectedId: string | null; - readonly onSelect: (id: string) => void; -}) { - return ( - - ); -} - -function ChannelEditor({ - tenantId, - channelId, - onSaved, -}: { - readonly tenantId: string; - readonly channelId: string; - readonly onSaved: (channel: ChannelSettings) => void; -}) { - const [state, setState] = useState>({ - kind: "loading", - }); - const [name, setName] = useState(""); - const [pinned, setPinned] = useState(false); - const [contextWindowInput, setContextWindowInput] = useState(""); - const [saving, setSaving] = useState(false); - const [saveError, setSaveError] = useState(null); - const [savedAt, setSavedAt] = useState(null); - - useEffect(() => { - let cancelled = false; - setState({ kind: "loading" }); - getChannelSettings(tenantId, channelId) - .then((settings) => { - if (cancelled) return; - setName(settings.title); - setPinned(settings.pinned); - const raw = rawContextWindow(settings); - setContextWindowInput(raw === undefined ? "" : String(raw)); - setState({ kind: "ready", data: settings }); - }) - .catch((cause: unknown) => { - if (!cancelled) { - setState({ kind: "error", message: errorMessage(cause) }); - } - }); - return () => { - cancelled = true; - }; - }, [tenantId, channelId]); - - if (state.kind === "loading") return ; - if (state.kind === "error") { - return ( - } - title={`Couldn't load ${SETTINGS_STRINGS.chatSettingsLoadError}`} - description={state.message} - /> - ); - } - - const parsedContextWindow = parseContextWindowInput(contextWindowInput); - const contextWindowValid = parsedContextWindow !== null; - const trimmedName = name.trim(); - const originalContextWindow = rawContextWindow(state.data); - const dirty = - trimmedName.length > 0 && - (trimmedName !== state.data.title || - pinned !== state.data.pinned || - (contextWindowValid && parsedContextWindow !== originalContextWindow)); - - function handleSave() { - if (!contextWindowValid || trimmedName.length === 0) return; - setSaving(true); - setSaveError(null); - patchChannelSettings(tenantId, channelId, { - "chat/name": trimmedName, - "chat/pinned": pinned, - ...(parsedContextWindow === undefined - ? {} - : { "chat/contextWindow": parsedContextWindow }), - }) - .then((updated) => { - setState({ kind: "ready", data: updated }); - onSaved(updated); - setSavedAt(new Date().toLocaleTimeString()); - }) - .catch(() => setSaveError(SETTINGS_STRINGS.chatSaveError)) - .finally(() => setSaving(false)); - } - - return ( - { - setName(state.data.title); - setPinned(state.data.pinned); - const raw = rawContextWindow(state.data); - setContextWindowInput(raw === undefined ? "" : String(raw)); - }} - /> - ); -} - /** - * The channel-settings form's markup on its own, taking already-resolved - * display fields — kept separate from `ChannelEditor` for the same reason + * The bench-defaults form's markup on its own, taking already-resolved + * display fields — kept separate from `ChatSection` for the same reason * `BenchSectionView` is: directly renderable in tests without a fetch stub. */ -export function ChannelEditorView({ - name, - pinned, +export function ChatSectionView({ contextWindowInput, contextWindowLabel: contextWindowLabelText, dirty, saving, error, savedAt, - onNameChange, - onPinnedChange, onContextWindowChange, onSave, onReset, }: { - readonly name: string; - readonly pinned: boolean; readonly contextWindowInput: string; readonly contextWindowLabel: string; readonly dirty: boolean; readonly saving: boolean; readonly error: string | null; readonly savedAt: string | null; - readonly onNameChange: (name: string) => void; - readonly onPinnedChange: (pinned: boolean) => void; readonly onContextWindowChange: (value: string) => void; readonly onSave: () => void; readonly onReset: () => void; @@ -216,24 +53,6 @@ export function ChannelEditorView({ savedAt={savedAt} onReset={onReset} > - - -

- {SETTINGS_STRINGS.chatPinnedDescription} -