diff --git a/.agents/skills/db-migrations/SKILL.md b/.agents/skills/db-migrations/SKILL.md index 70f432c16f..33707ef31b 100644 --- a/.agents/skills/db-migrations/SKILL.md +++ b/.agents/skills/db-migrations/SKILL.md @@ -6,6 +6,66 @@ user-invocable: false # Database Migrations Guide +## Development-stage schema changes + +Schema changes churn during feature development. When the schema changes before the migration has shipped, do not hand-edit the existing migration SQL to chase the new schema shape. Delete the draft migration artifacts added by this branch (SQL file, matching snapshot, and matching journal entry), then run the generator again and re-apply the normal migration review steps below. + +For example, if this branch's draft migration is `0110_add_verify_tables_and_ai_infra_id`: + +```bash +# 1. Delete the draft SQL and its snapshot +rm packages/database/migrations/0110_add_verify_tables_and_ai_infra_id.sql +rm packages/database/migrations/meta/0110_snapshot.json + +# 2. Remove the matching 0110 entry from the journal's "entries" array +# packages/database/migrations/meta/_journal.json + +# 3. Regenerate from the current schema +bun run db:generate +``` + +This keeps the generated SQL, snapshot, and journal aligned with the actual schema. Manual SQL edits are reserved for review-time hardening such as idempotent clauses, custom extension SQL, and meaningful filename/tag updates. + +Before release, if a feature branch accumulated multiple development-only migrations, consolidate them into one migration when possible. Production does not need to replay every intermediate draft shape, and fewer migrations reduce deploy-time risk. + +For example, if this branch added `0110`, `0111`, and `0112`, delete all three drafts and regenerate a single migration: + +```bash +# 1. Delete every draft SQL and snapshot this branch added +rm packages/database/migrations/011{0,1,2}_*.sql +rm packages/database/migrations/meta/011{0,1,2}_snapshot.json + +# 2. Remove the 0110/0111/0112 entries from the journal's "entries" array +# packages/database/migrations/meta/_journal.json + +# 3. Regenerate one migration covering the full schema delta +bun run db:generate +``` + +Do not make a migration compatible with earlier development-only versions of the same branch. While the migration has not shipped, there is no production history to preserve. Fix local/dev databases directly with whatever SQL is simplest (drop the draft table, rename a column, delete draft rows), then regenerate the branch migration from the current schema. + +For example, if an earlier draft on this branch created `signup_attempt_id` and you have since renamed it to `user_signup_log_id`, do not add a compatibility `ALTER ... RENAME` to the migration. Just fix the dev DB directly (see the `access-pg` skill for the `bun -e` + `pg` pattern), then regenerate: + +```bash +# Fix the dev DB to match the new schema (simplest SQL wins) +set -a && source .env && set +a && bun -e ' +import pg from "pg"; +const client = new pg.Client({ connectionString: process.env.DATABASE_URL }); +await client.connect(); +await client.query("ALTER TABLE user_signup_logs DROP COLUMN signup_attempt_id"); +await client.end(); +' + +# Regenerate so the migration reflects only the final shape +bun run db:generate +``` + +After a migration has reached production or the target default branch, treat it as immutable: add a follow-up migration instead of rewriting it. + +## Rebase conflicts + +When a rebase conflicts in migration files, keep the upstream/default-branch migrations and remove all migrations introduced by the current feature branch. Complete the rebase, then regenerate this branch's migration from the rebased schema. This avoids merging two independent snapshots or hand-splicing journal entries. + ## Step 1: Generate Migrations ```bash diff --git a/.agents/skills/drizzle/SKILL.md b/.agents/skills/drizzle/SKILL.md index cc81b84e3a..81d7229150 100644 --- a/.agents/skills/drizzle/SKILL.md +++ b/.agents/skills/drizzle/SKILL.md @@ -25,16 +25,42 @@ Location: `packages/database/src/schemas/_helpers.ts` - **Tables**: Plural snake_case (`users`, `session_groups`) - **Columns**: snake_case (`user_id`, `created_at`) +- **New tables**: Check nearby existing tables before naming a new one. Preserve + the established noun family and suffix. For example, if the user-scoped table + is `user_xxx_logs`, the workspace-scoped counterpart should be + `workspace_xxx_logs`, not `workspace_xxx_records` or another new synonym. + +```typescript +// ✅ Good: follows the existing user/workspace table family. +export const userSignupLogs = pgTable('user_signup_logs', { ... }); +export const workspaceSignupLogs = pgTable('workspace_signup_logs', { ... }); + +// ❌ Bad: introduces a new suffix for the same concept. +export const workspaceSignupRecords = pgTable('workspace_signup_records', { ... }); +``` ## Column Definitions ### Primary Keys +Do not use auto-incrementing primary keys (`serial`, `bigserial`, generated +identity columns). They create sequence-state problems during cross-database +migrations, restores, and data copy jobs. Prefer text IDs from application +generators (`idGenerator`, `createNanoId`) or `uuid` for internal tables. + +Keep `$defaultFn(...)` when a table normally owns ID generation. Callers can +still pass an explicit `id`; the default only runs when the insert omits it. Do +not remove the default just because one flow needs to supply a request-scoped ID. + ```typescript +// ✅ Good: app-generated text ID; explicit inserts can still override it. id: text('id') .primaryKey() .$defaultFn(() => idGenerator('agents')) .notNull(), + +// ❌ Bad: sequence state is fragile across DB migrations and restores. +id: serial('id').primaryKey(), ``` ID prefixes make entity types distinguishable. For internal tables, use `uuid`. @@ -53,6 +79,80 @@ userId: text('user_id') ...timestamps, // Spread from _helpers.ts ``` +### Optional and Undefined Values + +Do not introduce artificial sentinel strings for missing values, such as +`unknown`, unless the domain already has that explicit state and existing code +uses it consistently. Prefer nullable columns, optional TypeScript fields, or a +separate concrete status enum when the value is genuinely absent. + +```typescript +// ✅ Good: absent until the final stage writes a real decision. +export type UserSignupLogFinalDecision = 'allow' | 'block' | 'error'; + +finalDecision: varchar('final_decision', { length: 32 }).$type(), + +// ❌ Bad: invents a new state that callers now need to handle everywhere. +export type UserSignupLogFinalDecision = 'allow' | 'block' | 'error' | 'unknown'; + +finalDecision: varchar('final_decision', { length: 32 }) + .$type() + .notNull() + .default('unknown'); +``` + +### Field Descriptions + +For columns whose meaning is not obvious from the name alone, add JSDoc on the +schema field. Include a concrete example when it clarifies the stored value or +the lifecycle moment that writes it. This is especially important for external +IDs, lifecycle statuses, denormalized snapshots, JSONB signals, and fields whose +name could mean either a request ID or a persisted row ID. + +```typescript +// ✅ Good: explain the table's business object first, then only document +// non-obvious lifecycle or risk-control fields. +/** + * User signup logs - one row per signup flow, collecting stage-level + * risk-control decisions before and after the auth provider creates a user. + */ +export const userSignupLogs = pgTable('user_signup_logs', { + /** Final signup outcome reason, for example user_created, llm_block, or guard_error */ + finalReason: text('final_reason'), + + /** Aggregated risk level derived from stage decisions, for example block -> high */ + riskLevel: varchar('risk_level', { length: 16 }).$type(), + + /** Ordered stage-level decisions and metadata grouped by signup review stage */ + stageResults: jsonb('stage_results').$type(), +}); + +// ❌ Bad: comments restate obvious column names without adding domain meaning. +/** User email */ +email: text('email'), +``` + +### JSONB Types + +Avoid `Record` or similarly loose JSONB types for schema +columns. Define a concrete interface that describes the expected JSON shape, even +when most properties are optional. This keeps callers, migrations, and review +queries aligned on the same data contract. + +```typescript +interface UserSignupLogMetadata { + payloadPath?: string; + requestPath?: string; +} + +metadata: jsonb('metadata').$type(), +``` + +```typescript +// ❌ Bad: hides the contract and makes downstream access untyped. +metadata: jsonb('metadata').$type>(), +``` + ### Indexes ```typescript @@ -176,66 +276,52 @@ const rows = await this.db ### Raw SQL and Advanced Queries -Prefer Drizzle builders whenever the query can be expressed clearly with `select`, -`insert().select()`, `update().from()`, joins, CTEs, `groupBy`, and typed selected -columns. This keeps table and column references tied to schema definitions, so -schema changes are more likely to surface as TypeScript errors. +Prefer Drizzle builders whenever the query reads clearly with `select`, +`insert().select()`, `update().from()`, joins, CTEs, and `groupBy` — this keeps +table/column references tied to schema, so changes surface as TypeScript errors. +Within a builder, expression-level `sql` is fine for features lacking a helper +(JSON path, casts, aggregates, `CASE`, `NOW()`). Row locks are clauses, not +expressions — use `.for('update')`, never raw `FOR UPDATE`. -Expression-level `sql` is fine inside a Drizzle builder for PostgreSQL features -that do not have a dedicated helper, such as JSON path extraction, casts, aggregate -expressions, `CASE`, `NOW()`, or advisory locks. Row locks are query clauses, not -expressions; use the select builder's `.for('update')` instead of raw -`FOR UPDATE` SQL fragments. +Use `COALESCE` only when null-handling is part of required DB semantics (nullable +JSONB append/merge, "keep first non-null"). Don't scatter +`COALESCE(excluded.col, current.col)` across ordinary upsert scalars just to avoid +an update object — build `set` from defined values only, and hide any remaining +SQL behind named helpers (`appendJsonbArray`, `mergeJsonbObject`, `keepFirstValue`) +so the method reads as business intent, not SQL plumbing. + +```typescript +// ✅ Scalars included only when present; SQL hidden behind a named helper. +const updateValues = compactUndefined({ + email: record.email ?? undefined, + ip: record.ip ?? undefined, +}); +await db.insert(userSignupLogs).values(values).onConflictDoUpdate({ + set: { ...updateValues, stageResults: appendStageResult(stage, result), updatedAt: now }, + target: userSignupLogs.id, +}); + +// ❌ Every scalar becomes SQL plumbing. +set: { + email: sql`COALESCE(excluded.email, ${userSignupLogs.email})`, + ip: sql`COALESCE(excluded.ip, ${userSignupLogs.ip})`, +} +``` When refactoring raw SQL: -- Preserve the original query shape for latency-sensitive paths. If raw SQL is one - database roundtrip, do not replace it with multiple depth-based queries just to - remove `execute`. -- Use `$with(...)` plus `insert().select()` / `update().from()` for multi-step - single-roundtrip writes when Drizzle can express the data flow. -- Avoid generic `execute(sql...)` as the main safety mechanism. It types the - returned rows, but it does not keep selected columns in sync with schema changes. -- If the only clean implementation is a PostgreSQL feature that Drizzle cannot - express well, keep the raw SQL and tighten it instead: use schema references in - interpolations, explicit user scope, a narrow row interface, and regression tests. +- Preserve query shape on latency-sensitive paths. If raw SQL is one roundtrip, + don't split it into multiple depth-based queries just to drop `execute`. +- Use `$with(...)` + `insert().select()` / `update().from()` for multi-step + single-roundtrip writes Drizzle can express. +- Don't rely on `execute(sql...)` for safety — it types rows but doesn't keep + selected columns in sync with schema changes. +- If only a PostgreSQL feature Drizzle can't express works, keep the raw SQL and + tighten it: schema refs in interpolations, explicit user scope, a narrow row + interface, and regression tests. -Recursive CTEs are a special case: current Drizzle usage in this repo does not have -a clean `WITH RECURSIVE` builder pattern. Keep recursive CTE raw SQL when replacing -it would add extra database roundtrips or materially worsen performance. - -Example: convert an aggregate query when Drizzle can preserve one roundtrip: - -```typescript -// ✅ Good: builder owns table and column references; sql stays expression-level. -const rows = await trx - .select({ - model: messages.model, - provider: messages.provider, - totalCost: sql`sum((${messages.metadata}->'usage'->>'cost')::numeric)`.as( - 'totalCost', - ), - }) - .from(messages) - .where( - and( - eq(messages.topicId, topicId), - eq(messages.userId, userId), - eq(messages.role, 'assistant'), - sql`${messages.metadata} ? 'usage'`, - ), - ) - .groupBy(messages.provider, messages.model); -``` - -Example: use the select lock builder for row locks: - -```typescript -const [user] = await trx.select().from(users).where(eq(users.id, userId)).for('update'); -``` - -Example: keep a recursive CTE raw when replacing it would add depth-based DB -roundtrips: +Recursive CTEs are the canonical "keep raw" case — there's no clean `WITH RECURSIVE` +builder, and a rewrite would add depth-based roundtrips: ```typescript interface TaskTreeRow { @@ -243,15 +329,13 @@ interface TaskTreeRow { parent_task_id: string | null; } -// execute is acceptable here only because Drizzle has no clean WITH RECURSIVE -// builder; a builder rewrite would add depth-based roundtrips. Keep schema refs in -// the interpolations and scope every leg to the user. +// execute acceptable: no clean WITH RECURSIVE builder. Keep schema refs in the +// interpolations and scope every leg to the user. const { rows } = await db.execute(sql` WITH RECURSIVE task_tree AS ( SELECT ${tasks.id}, ${tasks.parentTaskId} FROM ${tasks} - WHERE ${tasks.id} = ${rootTaskId} - AND ${tasks.createdByUserId} = ${userId} + WHERE ${tasks.id} = ${rootTaskId} AND ${tasks.createdByUserId} = ${userId} UNION ALL SELECT ${tasks.id}, ${tasks.parentTaskId} FROM ${tasks} diff --git a/src/app/[variants]/(auth)/_layout/createAuthI18n.ts b/src/app/[variants]/(auth)/_layout/createAuthI18n.ts index eb519e8ceb..b5ab8a7f0a 100644 --- a/src/app/[variants]/(auth)/_layout/createAuthI18n.ts +++ b/src/app/[variants]/(auth)/_layout/createAuthI18n.ts @@ -107,6 +107,8 @@ export const createAuthI18n = (lang?: string) => { interpolation: { escapeValue: false }, keySeparator: false, lng: lang, + // Silence the Locize promotional console.info printed on init (i18next >= 25) + showSupportNotice: false, }); }, instance, diff --git a/src/components/Analytics/LobeAnalyticsProvider.tsx b/src/components/Analytics/LobeAnalyticsProvider.tsx index f4ae57ee8c..95766db308 100644 --- a/src/components/Analytics/LobeAnalyticsProvider.tsx +++ b/src/components/Analytics/LobeAnalyticsProvider.tsx @@ -12,7 +12,6 @@ import { memo, useRef } from 'react'; import { BUSINESS_LINE } from '@/const/analytics'; import { isDesktop } from '@/const/version'; -import { isDev } from '@/utils/env'; type Props = { children: ReactNode; @@ -32,7 +31,8 @@ export const LobeAnalyticsProvider = memo( analyticsInstance || createSingletonAnalytics({ business: BUSINESS_LINE, - debug: isDev, + // Keep the manager-level logs (`[AnalyticsManager] ...`) quiet even in dev + debug: false, providers: { ga4: ga4Config, posthog: postHogConfig, diff --git a/src/locales/create.ts b/src/locales/create.ts index 89f6f42742..6438d30360 100644 --- a/src/locales/create.ts +++ b/src/locales/create.ts @@ -110,6 +110,8 @@ export const createI18nNext = (lang?: string) => { keySeparator: false, lng: initialLang, + // Silence the Locize promotional console.info printed on init (i18next >= 25) + showSupportNotice: false, }); if (initialLang !== DEFAULT_LANG) {