From 1caf10ac857fb72cd5656c846f6cf1a2cd060a00 Mon Sep 17 00:00:00 2001 From: Andrew Kaiser Date: Mon, 30 Mar 2026 12:03:29 -0400 Subject: [PATCH] patterns and AGENTS.md updates --- AGENTS.md | 46 ++++++----- packages/core/.cursor/rules/core-patterns.md | 68 ++++++++++++++++ packages/web/.cursor/rules/web-patterns.md | 84 ++++++++++++++++++++ 3 files changed, 180 insertions(+), 18 deletions(-) create mode 100644 packages/core/.cursor/rules/core-patterns.md create mode 100644 packages/web/.cursor/rules/web-patterns.md diff --git a/AGENTS.md b/AGENTS.md index 51fb34fa..c92f72d8 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -442,29 +442,36 @@ await this.ctx.db.transaction_async(async () => { #### Creating Migrations 1. Create new file in `packages/core/src/db/migrations/` -2. Follow naming convention: `v{number}.ts` -3. Export migration object: +2. Follow naming convention: `migration_v{number}.ts` +3. Use the decorator class pattern: ```typescript -import { type Migration } from '@torm/sqlite' - -export const v9: Migration = { - version: 9, - up: (db) => { - db.exec(` - CREATE TABLE new_table ( - id INTEGER PRIMARY KEY, - name TEXT NOT NULL - ) - `) - }, - down: (db) => { - db.exec(`DROP TABLE new_table`) +import * as torm from '@torm/sqlite' +import { migrations, TIMESTAMP_COLUMN } from './registry.ts' + +@migrations.register() +export class Migration extends torm.Migration { + version = 12 + + call = () => { + this.driver.exec(`ALTER TABLE tag ADD COLUMN description TEXT`) } } ``` -4. Register in `packages/core/src/db/migrations/mod.ts` +4. Register in `packages/core/src/db/migrations/mod.ts` by adding import './migration_v{N}.ts' +5. Update the seed migration version to match: `version = {N}` in `seed_migration.ts` +6. Update the seed migration's `CREATE TABLE` statements to include the new schema +7. Update `test/migrations.test.ts`: bump `CURRENT_VERSION` and add a new migration step to `expected_migration_operations` + +##### SQLite Migration Pitfalls + +- `DROP COLUMN` fails when the column has a `FOREIGN KEY` constraint. You must rebuild the table: create a new table without the column, copy data, drop old table, rename new table. This also requires dropping and recreating all triggers that reference the table. +- `PRAGMA foreign_keys = OFF` cannot be set inside a transaction. When rebuilding tables, set `override TRANSACTION = false` on the migration class and manage `BEGIN TRANSACTION / COMMIT` manually. +- **Triggers reference tables by name**. When you drop and recreate a table (even via rename), all triggers that reference it must be dropped first and recreated after, including triggers defined on other tables that reference the rebuilt table in their body. +- `PRAGMA foreign_key_check` should be called before `COMMIT` when rebuilding tables with FK constraints disabled. +- The seed migration schema must match the migrated schema exactly. The migration test (`test/migrations.test.ts`) compares a freshly seeded database against a database migrated from v1. If the schemas differ (e.g., `ALTER TABLE ADD COLUMN` stores FK constraints differently than `CREATE TABLE`), use the table rebuild pattern instead. + ### Working with Media Files @@ -713,6 +720,8 @@ core: 9. **Use comments where applicable** - But not liberally; code should be self-documenting 10. **Core actions interfaces should have docstrings** - Document public API methods 11. **PR titles must follow main's commit-title pattern** - Use lowercase Conventional Commit style, typically `(): ` (e.g. `fix(web): ...`, `impl(core): ...`, `docs: ...`) +12. **Use semantic field names** - Avoid generic names like `source`/`target` in database columns and interfaces. Use names that describe what the field represents (e.g. `alias_tag_slug`/`alias_for_tag_slug` instead of `source_tag_slug`/`target_tag_slug`) +13. **Consider side effects of cleanup operations** - When tags or other entities can be auto-cleaned (e.g. `Tag.delete_unreferenced`), ensure rules or references that depend on them are accounted for. Cleanup queries should exclude entities that are referenced by rules. ### Security Considerations @@ -723,10 +732,11 @@ core: ### Performance Considerations -1. **Database transactions** - Use for multiple operations +1. **Database transactions** - Use for multiple operations. Wrap related create/delete sequences in `transaction_sync`. 2. **Thumbnail generation** - Async and cached 3. **Large file handling** - Stream when possible 4. **Query optimization** - Use indexes, limit results +5. **Use `COUNT` queries** - When you only need to check existence or get a count, use a `SELECT COUNT(1)` query instead of fetching all rows and checking `.length`. See `TagAlias.count_by_alias` and `TagParent.count_children` for examples. ### Breaking Changes diff --git a/packages/core/.cursor/rules/core-patterns.md b/packages/core/.cursor/rules/core-patterns.md new file mode 100644 index 00000000..8dff7779 --- /dev/null +++ b/packages/core/.cursor/rules/core-patterns.md @@ -0,0 +1,68 @@ +# @forager/core Patterns + +## Database Migrations + +Migrations use the `@migrations.register()` decorator pattern. See `packages/core/src/db/migrations/migration_v10.ts` for a complex example involving table rebuilds with trigger management. + +When creating a new migration: +- The seed migration version (`seed_migration.ts`) must be bumped to match +- The seed migration's `CREATE TABLE` statements must reflect the final schema +- `packages/core/src/db/migrations/mod.ts` must import the new file +- `test/migrations.test.ts` must be updated with the new version and migration step + +### Table Rebuild Pattern + +When `ALTER TABLE` is insufficient (e.g. dropping columns with FK constraints, adding FK columns that must match the seed schema format): + +1. Set `override TRANSACTION = false` on the migration class +2. `PRAGMA foreign_keys = OFF` +3. `BEGIN TRANSACTION` +4. Drop ALL triggers referencing the table (including triggers on other tables that reference this one in their body) +5. `CREATE TABLE new_table (...)` with desired schema +6. `INSERT INTO new_table ... SELECT ... FROM old_table` +7. `DROP TABLE old_table` +8. `ALTER TABLE new_table RENAME TO old_table` +9. Recreate all indexes +10. Recreate all triggers +11. `PRAGMA foreign_key_check` +12. `COMMIT` +13. `PRAGMA foreign_keys = ON` + +## Actions Pattern + +### `media_add_tag` Helper + +The `media_add_tag` method in `actions/lib/base.ts` is the single entry point for adding a tag to a media reference. It handles: +- Tag creation/lookup via `tag_create` +- Alias resolution (checks `tag_alias` table, follows to canonical tag) +- Media reference tag creation (with `tag_alias_id` tracking) +- Parent tag propagation (walks `tag_parent` chain via BFS, creates rows with `tag_parent_id` tracking) + +All callers should use `media_add_tag` instead of manually calling `tag_create` + `MediaReferenceTag.get_or_create`. + +### Tag Rule Undo + +When creating alias/parent rules, the `tag_alias_id` and `tag_parent_id` columns on `media_reference_tag` track which rule caused each row to be created. When a rule is deleted: +- `alias_delete`: migrates tracked rows back to the original alias tag (true undo) +- `parent_delete`: deletes tracked rows (removes the parent tag that was auto-applied) +- Rows created manually (without a rule) have `null` for these columns and are never affected by rule deletion. + +## Model Patterns + +### Count Queries + +When only a count is needed (not the full row data), use `SELECT COUNT(1)` queries with `PaginationVars.result.total`: + +```typescript +#count_by_alias = this.query` + SELECT COUNT(1) AS ${PaginationVars.result.total} FROM tag_alias + WHERE alias_tag_slug = ${TagAlias.params.alias_tag_slug}` + +public count_by_alias(params: { alias_tag_slug: string }): number { + return this.#count_by_alias.one(params)!.total +} +``` + +### Slug-Based References + +The `tag_alias` and `tag_parent` tables reference tags by slug (e.g. `genre:adventure`) rather than by ID foreign key. This allows rules to exist before the tags they reference are created in the database, and to survive tag deletion/recreation cycles from auto-cleanup. diff --git a/packages/web/.cursor/rules/web-patterns.md b/packages/web/.cursor/rules/web-patterns.md new file mode 100644 index 00000000..220d8280 --- /dev/null +++ b/packages/web/.cursor/rules/web-patterns.md @@ -0,0 +1,84 @@ +# @forager/web Patterns + +## Svelte 5 Runes + +### File Extensions + +Files that use `$state`, `$derived`, `$effect`, or other Svelte 5 runes **must** use the `.svelte.ts` extension (not plain `.ts`). This includes controller files and rune files. Using `$state` in a `.ts` file will cause the runtime error: + +``` +Uncaught Svelte error: rune_outside_svelte +The `$state` rune is only available inside `.svelte` and `.svelte.js/ts` files +``` + +Examples of files that need `.svelte.ts`: +- `controller.svelte.ts` (uses `$state` for reactive properties) +- `runes/queryparams.svelte.ts` (uses `$state` for search results) + +Files that do NOT need `.svelte.ts`: +- `controller.ts` files that don't use any runes (e.g. `packages/web/src/routes/tags/controller.ts`) + +## SvelteKit Navigation Patterns + +### Page Param Reactivity + +When a SvelteKit page component needs to react to route param changes (e.g. `/tags/[slug]`), use `{#key}` to fully reset the page: + +```svelte + + +{#key slug} + {@const controller = new Controller(props.data.config, slug)} + +{/key} +``` + +**Do NOT use `onMount`** for loading data based on route params — it only fires once when the component mounts. Client-side navigations between different params (e.g. clicking a link from `/tags/foo` to `/tags/bar`) reuse the same component instance without re-running `onMount`. + +**`$effect` is a partial solution** — it re-runs the data fetch but doesn't reset form inputs, component state, or child component state. `{#key}` destroys and recreates everything inside it, giving a clean slate. + +## Controller Pattern + +Controllers extend `BaseController` and are instantiated per-page. They hold: +- `runes` object with reactive state (focus, settings, queryparams) +- Handler methods for RPC calls +- The RPC `client` (inherited from `BaseController`) + +For pages with URL-synced search state, use `BaseQueryParams` from `$lib/runes/base_queryparams.svelte.ts`. + +## CSS Patterns + +### `display: contents` for Grid-Through-Slot Layouts + +When a parent uses CSS grid and children are rendered via Svelte slots/snippets, `display: contents` on the wrapper makes the slot's children become direct grid items: + +```svelte + +
+ +
+ {@render children?.()} +
+
+ + +
+
+
+
+``` + +## URL Encoding + +Tag slugs contain only `[a-z0-9_:]` which are all valid URL path characters. Do **not** use `encodeURIComponent` for tag slugs in URLs — it encodes `:` to `%3A` which looks wrong. Use the slug directly: + +```svelte + + + + + +```