Skip to content

Conversation

anettkva
Copy link
Member

Summary

Adds the Prisma schema for the whole data model and the first migration, which creates all the tables in PostgreSQL. The search follows the design in docs/API.md ("Search without raw SQL"), with a few changes described below.

Changes

  • Catalog tables: apparatus (with position for the sort order), element_groups, variant_groups, elements and vaults. The catalog keeps the readable ids from data/.
  • User data tables: coaches, gymnasts and routines (with created_at and updated_at). These get generated ids (cuid).
  • Lists are stored as rows:
    • a routine's items are in routine_elements, with a position that is unique per routine
    • a gymnast's element list is in gymnast_elements, with no duplicates per gymnast
    • each row points to either an element or a vault, and a CHECK constraint makes sure exactly one of them is set
  • Variants: elements and vaults have a variant_group_id instead of a list on the variant group.
  • Values: value and d_value are Decimal(3, 1), so summing D-values has no rounding errors.
  • Removed elements: elements and vaults have removed_at, so they can be hidden without being deleted.
  • On delete: deleting a coach deletes their gymnasts, and deleting a gymnast deletes her routines and list. Catalog rows that are in use cannot be deleted.
  • Search tables: catalog_entries and catalog_words, with the indexes from API.md.
  • Docs:
    • docs/DATA_MODEL.md has a new "Storage in PostgreSQL" section and updated diagram, tables and constraint numbers, and explains why vault is its own table
    • the README has the new npm run db:migrate step
    • the comment on the search argument in schema.graphql is updated

Changes to the search design in docs/API.md

The design is the one in "Search without raw SQL". These are the differences:

  1. The C collation is set on the word column only, not on the whole database. API.md first suggested creating the database with the C collation. Instead the migration sets it on catalog_words.word, and schema.prisma has an ordinary @@index([word]).
    • Our databases use en-US, so an ordinary index is not used for a prefix search (startsWith). We checked this with EXPLAIN.
    • A database-wide C collation would change the sorting of all text, including nameSort for names with accents. With the column setting, everything else keeps en-US.
    • With a database-wide setting, anyone who created the database with the default settings would silently get a slow search. The column setting is part of the migration, so it works the same on every machine and on the VM.
  2. Why not text_pattern_ops. We tried it on the index first. Prisma accepts it in schema.prisma, but it can't read it back from the database, so every prisma migrate dev wanted to drop and recreate the index.
  3. CatalogEntry has one row per code, the -a variant, because the catalog list shows one row per code. CatalogWord holds the words of all variants of the code, so a word that is only in variant -b still finds it. The seed script will also add written-out abbreviations (for example handstand for hstd).
  4. Several search words must all match, and a trailing "s" is removed from each search word, so "saltos" finds "salto".
  5. The search must never use mode: 'insensitive'. It turns LIKE into ILIKE, and then the index isn't used. The words are stored in lowercase, and the search term is lowercased before the query.

How to test

  1. Pull the branch and run npm ci from the root
  2. Run npm run db:migrate to create the tables in your local database
  3. Run cd backend and then npx prisma migrate status. It should end with "Database schema is up to date!"
  4. Run npx prisma migrate dev in backend/. It should say "Already in sync", which means the schema and the migration match

Closes #22

🤖 Generated with Claude Code

Anettkva added 4 commits October 15, 2026 09:26
All tables of the data model, with join tables for routines and
gymnast element lists, CatalogEntry and CatalogWord for the search,
and CHECK constraints and the C collation on catalog_words.word
added in SQL in the migration.
One catalog entry per code with the words of all variants, several
search words that must all match, a trailing "s" removed, and the C
collation on the word column instead of a database-wide setting.
@anettkva anettkva requested a review from a team as a code owner October 15, 2026 07:33
@ronjalh
Copy link
Member

ronjalh commented Oct 15, 2026

Review 1/4: Schema and indexes

Nice work. I applied the migration to a fresh PostgreSQL 18: migrate deploy, migrate status and migrate dev ("Already in sync") all pass, and the CHECK, unique, RESTRICT and CASCADE rules behave as documented. The C collation also works: LIKE 'sal%' becomes an index range scan.

The numbers below are from a local test database with synthetic data about 400× the real catalog (225 000 entries), queried through Prisma. They show the shape of the problem, not exact timings.

Priority: High = fix before merge, Medium = fix or decide soon, Low = nice to have.

1. Name sort ascending cannot use its index. High
@@index([hasName, nameSort, id]) is all ascending, but the sort is hasName DESC, nameSort ASC, id ASC, so only the DESC sort is served.

  • name ASC, page 32 via cursor: 54 ms without a search and 290 ms with "sal". name DESC: 1–5 ms. The cost grows with the catalog.
  • Fix (tested, 82 ms → 0.05 ms): add @@index([hasName(sort: Desc), nameSort, id]) and keep the existing one.
  • Alternative: drop hasName from the sort and let unnamed items reverse with the direction. One index, but a product choice.

2. Filter + difficulty/code sort is slow in one direction. High
VT + difficulty ASC takes 150 ms (DESC: 0.2 ms), because vaults have the highest difficulty and the walk passes all other rows first.

  • Fix (tested, both directions ~0.05 ms): @@index([apparatusId, difficulty, id]) and @@index([apparatusId, codeSort, id]).

3. What does DIFFICULTY mean? High
API.md says value for elements and dValue for vaults in one list, but the ranges do not mix (0.1–1.0 vs about 2.0–6.0). #24 says "within each apparatus". Please pick one and align API.md, schema.graphql and the indexes (item 2 fits the per-apparatus option).

4. Routine.updatedAt. Medium
Items are separate rows, so @updatedAt does not change when setRoutineItems runs. Touch it in the same transaction. The column also has no database default, so inserts outside Prisma fail with NOT NULL; @default(now()) next to @updatedAt fixes that.

Comment made with Claude Code in discussion with Ronja

@ronjalh
Copy link
Member

ronjalh commented Oct 15, 2026

Review 2/4: Search and backend notes

These affect #23 and #24 more than this PR, but they follow from the schema, so I am listing them here. Same test setup as in comment 1/4.

Priority: High = fix before merge, Medium = fix or decide soon, Low = nice to have.

1. Wildcards are not escaped. High (for #24)
Prisma passes % and _ through in startsWith: startsWith: "%" matched all 224 800 entries (464 ms). The cleaning to letters, digits and . is the only protection. Please add tests for %, _ and uppercase input ("SAL" returns 0 hits, so lowercasing is required).

2. totalCount is O(n). Medium
count took 33 ms for 225 000 rows and 107 ms for the "sal" search. If the UI always asks for it, every keystroke pays. Suggest asking only on the first page, or capping the count ("1000+").

3. Decimal values. Medium
Prisma returns Decimal objects, not numbers. Sum the D-values as Decimal and convert at the end. Converting to Number first loses the exact sums that "Numbers" in DATA_MODEL.md promises.

4. Seed. Medium
catalog_entries has no foreign keys to elements, vaults or groups (I could insert an orphan row), so the seed should rebuild entries and words in one transaction. The files in data/ also still have the old shape (variantIds lists, no variantGroupId or removedAt), so the seed has to convert them. Worth a line in #23.

5. Generic query plans. Low
Prisma sends word::text LIKE ($1 || '%'). With normal plans the index is used (word >= 'sal' AND word < 'sam', 0.03 ms). With plan_cache_mode = force_generic_plan it becomes a sequential scan (86 ms on 3 million rows). Add a note to API.md: do not set it, and be careful with a pooler that uses named prepared statements.

Comment made with Claude Code in discussion with Ronja

@ronjalh
Copy link
Member

ronjalh commented Oct 15, 2026

Review 3/4: Open questions

Things I could not tell from the PR or the docs. Answers can be a sentence in DATA_MODEL.md or a link to an issue.

Priority: High = needs an answer before merge, Medium = decide soon, Low = nice to have.

1. The gymnast's element list has no GraphQL field. High
GymnastElement has a table and is in the README's planned functionality ("keep a list of the elements each gymnast can do or is practicing"), but schema.graphql has no field or mutation for it. Is it planned for a later issue? If so, please link it.

2. Removed elements. Medium

  • If -a is removed but -b is still valid, the code disappears from the catalog, because CatalogEntry is always -a. Should it use the first variant that is not removed?
  • What do element(id), catalogItem(id) and variants return for a removed item?
  • What does a routine that contains a removed element show (D-value, and a warning in the UI)?

3. Duplicates and positions in a routine. Medium
The database accepts the same element twice in one routine (different position) and a position of 0 or negative. Is that intended? The D-value counts each code once, but normally the same element is not repeated in a routine. If duplicates are not allowed, the backend needs to check it.

4. Value ranges and limits. Low
There is no CHECK on value (0.1–1.0) or d_value; I could insert an element with value 5.0 and −1. Is that left to the extraction script on purpose? The limits (200 gymnasts, 50 routines, 50 items) are only in the backend, so two simultaneous requests can exceed them. Probably fine here, but worth a line.

Comment made with Claude Code in discussion with Ronja

@ronjalh
Copy link
Member

ronjalh commented Oct 15, 2026

Review 4/4: Docs and process

Priority: High = fix before merge, Medium = fix or decide soon, Low = nice to have.

Medium

Low

  • docs/examples/ is outdated. It still has variantIds lists, routines with elementIds, no position, no gymnast element list and no removedAt, but DATA_MODEL.md links to it as examples of how each entity is structured.
  • Calculator. DATA_MODEL.md says the query takes apparatusId and elementIds; schema.graphql has calculateDScore(itemIds). Which one is intended?
  • Variants. DATA_MODEL.md says each variant "can be searched and added to a routine on its own", but the search now returns only the -a variant per code.
  • Diagram. It matches schema.prisma for the 10 tables it shows (I compared every field and relation). CatalogEntry and CatalogWord are left out on purpose; a small second diagram (CATALOG_ENTRY ||--o{ CATALOG_WORD) would show everything in the database.
  • Wording. DATA_MODEL.md and the PR text say "her routines" and "she can do" about gymnasts. "Their" is safer, since nothing says the gymnast's gender.
  • Base branch. The PR targets dev, not main. Is that intended?

Comment made with Claude Code in discussion with Ronja

@anettkva
Copy link
Member Author

anettkva commented Oct 15, 2026 •

Thanks for the thorough reviews! Here is what I did with each point.

Review 1: indexes, difficulty and timestamps

  1. Name sort ascending: fixed. Added @@index([hasName(sort: Desc), nameSort, id]) and kept the existing index.
  2. Filter + difficulty/code sort: fixed. Added @@index([apparatusId, difficulty, id]) and @@index([apparatusId, codeSort, id]). Checked with EXPLAIN on 225 000 synthetic rows: each query uses its index and takes under 0.4 ms.
  3. What DIFFICULTY means: decided. Sorting by difficulty is only allowed when an apparatus is selected, otherwise BAD_USER_INPUT, because comparing element values with vault D-values gives no useful order. We also replaced minDifficulty/maxDifficulty with difficultyValues, a list of chosen values. Letters (A–J) are only shown in the frontend. Documented in API.md and DATA_MODEL.md, and schema.graphql is updated.
  4. Routine.updatedAt: fixed. It has @default(now()), and DATA_MODEL.md says that setRoutineItems must update it in the same transaction.

Review 2: search safety and backend rules

  1. Wildcards: API.md now says that Prisma does not escape % and _, so cleaning each search word is the only protection. Tests for %, _ and uppercase input are added to Add indexes and search #24.
  2. totalCount: the client asks for it only with the first page of a new search or filter. A capped count is noted as the next step if needed.
  3. Decimal: DATA_MODEL.md says D-values are added up as Decimal and converted only at the end.
  4. Seed: DATA_MODEL.md says the search tables are rebuilt in one transaction, and Seed the catalog #23 covers converting the old shape of data/.
  5. Generic query plans: added a note to API.md: do not set plan_cache_mode = force_generic_plan, and be careful with poolers that use named prepared statements.

Review 3: open questions

  1. The gymnast's element list: added to the API now: Gymnast.elements, addGymnastElement and removeGymnastElement. Adding an item twice or removing a missing one does nothing.
  2. Removed elements:
    • CatalogEntry uses the first variant that is not removed, so a code with a removed -a is still found through -b
    • removed items can still be fetched by id, with a new removed field
    • variants leaves them out
    • in routines and gymnast lists they stay with a warning in the UI and do not count in the D-value
  3. Duplicates in a routine: allowed on purpose. The element still counts once, but connection bonuses we plan to add later (for example C + C) can involve the same element twice. Position now has CHECK (position >= 1).
  4. Ranges and limits: added CHECKs for value (0.1–1.0) and d_value (> 0). The limits stay backend-only, and going slightly over with concurrent requests is accepted. Documented in DATA_MODEL.md.

All decisions from this review have their own sections under "Design decisions" in DATA_MODEL.md.

Review 4: docs and issues

  • Create the database schema #22: updated to what was built.
  • AI usage: added entries for the schema and migration work and for the extraction script.
  • Abbreviations: Seed the catalog #23 now has a check that every abbreviation in the data is covered, and Add indexes and search #24 has a test that "handstand" finds an element described with "hstd".
  • docs/examples/: updated to the new storage shape, with two new files for the gymnast lists and the routine items, and the real group names.
  • Calculator: calculateDScore(itemIds) is the intended one. DATA_MODEL.md now matches it.
  • Variants: the text now says the search finds the code, and the user chooses the variant on the detail page.
  • Diagram: added a second diagram for CatalogEntry and CatalogWord.
  • Wording: "she" is intended. The app is built on the Code of Points for women's artistic gymnastics, so all gymnasts are women. This is now stated in the introduction of DATA_MODEL.md.
  • Base branch: intended. Feature branches are merged into dev, and dev into main when we deliver.

I also updated the README and API.md so they describe the current state.

Copy link
Member

@ronjalh ronjalh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm

@ronjalh ronjalh merged commit d007b13 into dev Oct 15, 2026
Sign in to join this conversation on GitHub.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants