diff --git a/backend/src/dscore.test.ts b/backend/src/dscore.test.ts new file mode 100644 index 0000000..5d94661 --- /dev/null +++ b/backend/src/dscore.test.ts @@ -0,0 +1,114 @@ +// Tests for the D-score function. No database is needed. + +import { describe, expect, it } from 'vitest' +import { calculateDScore, type ScoredItem } from './dscore.js' +import { InvalidItemsError } from './errors.js' + +const element = ( + code: string, + value: string, + apparatus = 'UB', +): ScoredItem => ({ + id: `${apparatus}-${code}-a`, + code, + value, + isVault: false, +}) +const vault = (code: string, value: string): ScoredItem => ({ + id: `VT-${code}-a`, + code, + value, + isVault: true, +}) + +describe('calculateDScore', () => { + it('gives 0 for an empty list', () => { + expect(calculateDScore([])).toBe(0) + }) + + it('sums all the elements when there are fewer than eight', () => { + expect( + calculateDScore([element('1.101', '0.1'), element('2.102', '0.5')]), + ).toBe(0.6) + }) + + it('sums exactly eight elements', () => { + const items = Array.from({ length: 8 }, (_, i) => + element(`3.${300 + i}`, '0.3'), + ) + expect(calculateDScore(items)).toBe(2.4) + }) + + it('counts only the eight highest of more than eight', () => { + const values = [ + '1.0', + '0.9', + '0.8', + '0.7', + '0.6', + '0.5', + '0.4', + '0.3', + '0.2', + '0.1', + ] + const items = values.map((v, i) => element(`4.${400 + i}`, v)) + // 1.0 + 0.9 + 0.8 + 0.7 + 0.6 + 0.5 + 0.4 + 0.3 = 5.2; 0.2 and 0.1 are left out + expect(calculateDScore(items)).toBe(5.2) + }) + + it('counts each code once, also when two variants are in the list', () => { + const a = element('1.101', '0.4') + const b = { ...element('1.101', '0.4'), id: 'UB-1.101-b' } + expect(calculateDScore([a, b])).toBe(0.4) + // The same code does not use up two of the eight places. + const others = Array.from({ length: 7 }, (_, i) => + element(`5.${500 + i}`, '0.1'), + ) + expect(calculateDScore([a, b, ...others])).toBe(1.1) + }) + + it('does not get rounding errors from decimals', () => { + // As floats 0.1 + 0.2 is 0.30000000000000004. + expect( + calculateDScore([element('1.101', '0.1'), element('1.102', '0.2')]), + ).toBe(0.3) + const items = Array.from({ length: 8 }, (_, i) => + element(`6.${600 + i}`, '0.7'), + ) + expect(calculateDScore(items)).toBe(5.6) + }) + + it('accepts values that are not strings, such as Prisma decimals', () => { + const decimal = { toString: () => '0.8' } + expect( + calculateDScore([{ ...element('1.101', '0'), value: decimal }]), + ).toBe(0.8) + }) + + it('gives the D-value of the vault on vault', () => { + expect(calculateDScore([vault('4.1', '2.6')])).toBe(2.6) + // Two variants of the same vault are one vault. + expect( + calculateDScore([ + vault('4.1', '2.6'), + { ...vault('4.1', '2.6'), id: 'VT-4.1-b' }, + ]), + ).toBe(2.6) + }) + + it('refuses more than one vault', () => { + expect(() => + calculateDScore([vault('4.1', '2.6'), vault('5.1', '3.0')]), + ).toThrow(InvalidItemsError) + }) + + it('refuses items from more than one apparatus, also a vault with elements', () => { + expect(() => + calculateDScore([element('1.101', '0.1'), element('1.102', '0.1', 'BB')]), + ).toThrow(InvalidItemsError) + expect(() => + calculateDScore([vault('4.1', '2.6'), element('1.101', '0.1')]), + ).toThrow(InvalidItemsError) + }) +}) diff --git a/backend/src/dscore.ts b/backend/src/dscore.ts new file mode 100644 index 0000000..2e38e07 --- /dev/null +++ b/backend/src/dscore.ts @@ -0,0 +1,60 @@ +// The D-score of a list of elements or vaults (constraint 13 in +// docs/DATA_MODEL.md). The calculator (`calculateDScore`) and, later, +// `Routine.dScore` use this function, so both follow the same rule. +// +// - Elements: the sum of the eight highest values, where each code counts once. +// Fewer than eight elements are all summed. An empty list gives 0. +// - Vault: the routine holds one vault and the score is that vault's D-value. +// +// The values have one decimal, so the sum is made in tenths (whole numbers) and +// divided at the end. Adding 0.1 + 0.2 as decimals gives 0.30000000000000004. + +import { InvalidItemsError } from './errors.js' +import { apparatusOfId } from './server/items.js' + +/** How many elements count in the score. */ +export const COUNTED_ELEMENTS = 8 + +export type ScoredItem = { + id: string + code: string + /** `value` of an element, `dValue` of a vault. A string or Prisma Decimal with one decimal. */ + value: { toString(): string } + isVault: boolean +} + +const toTenths = (value: { toString(): string }): number => + Math.round(Number(value.toString()) * 10) + +/** + * The D-score of the items. Throws InvalidItemsError when they do not fit + * together: items from more than one apparatus (this includes a vault with + * elements), or more than one vault. + */ +export function calculateDScore(items: ScoredItem[]): number { + if (new Set(items.map((item) => apparatusOfId(item.id))).size > 1) { + throw new InvalidItemsError( + 'The items must all be on the same apparatus, and a vault cannot be combined with elements.', + ) + } + + // One value for each code. Variants of a code have the same value, so the + // highest is the same as any. + const byCode = new Map() + for (const item of items) { + const value = toTenths(item.value) + byCode.set(item.code, Math.max(byCode.get(item.code) ?? 0, value)) + } + + if (items.some((item) => item.isVault)) { + if (byCode.size > 1) { + throw new InvalidItemsError('A routine on vault has only one vault.') + } + return [...byCode.values()][0]! / 10 + } + + const highest = [...byCode.values()] + .sort((a, b) => b - a) + .slice(0, COUNTED_ELEMENTS) + return highest.reduce((sum, tenths) => sum + tenths, 0) / 10 +} diff --git a/backend/src/errors.ts b/backend/src/errors.ts index fe22bc4..1cd1105 100644 --- a/backend/src/errors.ts +++ b/backend/src/errors.ts @@ -39,14 +39,32 @@ export class NotImplementedError extends Error { } } +/** + * The items do not fit together: items from more than one apparatus, a vault + * with elements, more than one vault, or an item that has been removed from + * the Code of Points. Code "INVALID_ITEMS". + */ +export class InvalidItemsError extends Error { + readonly code = 'INVALID_ITEMS' + + constructor(message: string) { + super(message) + this.name = 'InvalidItemsError' + } +} + /** The errors above, which the server shows to the client with their code. */ export type ClientError = - BadUserInputError | UnauthenticatedError | NotImplementedError + | BadUserInputError + | UnauthenticatedError + | NotImplementedError + | InvalidItemsError export function isClientError(error: unknown): error is ClientError { return ( error instanceof BadUserInputError || error instanceof UnauthenticatedError || - error instanceof NotImplementedError + error instanceof NotImplementedError || + error instanceof InvalidItemsError ) } diff --git a/backend/src/server/calculator.test.ts b/backend/src/server/calculator.test.ts new file mode 100644 index 0000000..e221325 --- /dev/null +++ b/backend/src/server/calculator.test.ts @@ -0,0 +1,175 @@ +// Tests for the calculator query. They run against the local database from +// backend/.env and need the seeded catalog (run `npm run db:seed` first). They +// only read. Without DATABASE_URL they are skipped. + +import type { ApolloServer } from '@apollo/server' +import { afterAll, beforeAll, describe, expect, it } from 'vitest' +import { calculateDScore } from '../dscore.js' +import { createPrismaClient } from '../db.js' +import type { PrismaClient } from '../generated/prisma/client.js' +import { createApolloServer } from './app.js' +import type { Config } from './config.js' +import { type Context, createContext } from './context.js' +import { MAX_CALCULATOR_IDS } from './resolvers/calculator.js' + +const hasDatabase = Boolean(process.env['DATABASE_URL']) +const config: Config = { + port: 3001, + imageBaseUrl: 'http://images.test/images', + corsOrigins: [], +} + +type GqlError = { message: string; extensions?: Record } +type Result = { + data: { calculateDScore: number } | null | undefined + errors: readonly GqlError[] | undefined +} + +const QUERY = 'query ($ids: [ID!]!) { calculateDScore(itemIds: $ids) }' + +describe.runIf(hasDatabase)('calculateDScore', () => { + let prisma: PrismaClient + let server: ApolloServer + + const run = async ( + ids: string[], + client: PrismaClient = prisma, + ): Promise => { + const response = await server.executeOperation( + { query: QUERY, variables: { ids } }, + { contextValue: createContext(client, config) }, + ) + if (response.body.kind !== 'single') throw new Error('expected one result') + const { data, errors } = response.body.singleResult + return { data: data as Result['data'], errors } + } + const codeOf = (r: Result) => r.errors?.[0]?.extensions?.['code'] + + beforeAll(async () => { + prisma = createPrismaClient() + server = createApolloServer() + await server.start() + }) + + afterAll(async () => { + await server.stop() + await prisma.$disconnect() + }) + + /** The values of the first n elements on an apparatus, from the database. */ + async function elements(apparatusId: string, n: number) { + const rows = await prisma.element.findMany({ + where: { id: { startsWith: `${apparatusId}-` }, removedAt: null }, + orderBy: { id: 'asc' }, + }) + const seen = new Set() + return rows.filter((r) => !seen.has(r.code) && seen.add(r.code)).slice(0, n) + } + + it('is open to everyone: no login is needed', async () => { + const result = await run([]) + expect(result.errors).toBeUndefined() + expect(result.data!.calculateDScore).toBe(0) + }) + + it('gives the same score as the function for elements of the catalog', async () => { + const rows = await elements('UB', 10) + const expected = calculateDScore( + rows.map((r) => ({ + id: r.id, + code: r.code, + value: r.value, + isVault: false, + })), + ) + const result = await run(rows.map((r) => r.id)) + expect(result.errors).toBeUndefined() + expect(result.data!.calculateDScore).toBe(expected) + expect(expected).toBeGreaterThan(0) + }) + + it('gives the D-value of a vault', async () => { + const vault = await prisma.vault.findFirstOrThrow({ + orderBy: { id: 'asc' }, + }) + const result = await run([vault.id]) + expect(result.data!.calculateDScore).toBe(Number(vault.dValue.toString())) + }) + + it('counts the same id twice as once, and a variant of the same code once', async () => { + const [first] = await elements('BB', 1) + const once = await run([first!.id]) + const twice = await run([first!.id, first!.id]) + expect(twice.data!.calculateDScore).toBe(once.data!.calculateDScore) + }) + + it('refuses items that do not fit together with INVALID_ITEMS', async () => { + const [ub] = await elements('UB', 1) + const [bb] = await elements('BB', 1) + const vault = await prisma.vault.findFirstOrThrow({ + orderBy: { id: 'asc' }, + }) + const second = await prisma.vault.findFirstOrThrow({ + where: { code: { not: vault.code } }, + orderBy: { id: 'asc' }, + }) + expect(codeOf(await run([ub!.id, bb!.id]))).toBe('INVALID_ITEMS') + expect(codeOf(await run([ub!.id, vault.id]))).toBe('INVALID_ITEMS') + expect(codeOf(await run([vault.id, second.id]))).toBe('INVALID_ITEMS') + }) + + it('refuses a removed item with INVALID_ITEMS', async () => { + const [row] = await elements('UB', 1) + const fake = { + element: { + findMany: () => Promise.resolve([{ ...row!, removedAt: new Date() }]), + }, + vault: { findMany: () => Promise.resolve([]) }, + } as unknown as PrismaClient + const result = await run([row!.id], fake) + expect(codeOf(result)).toBe('INVALID_ITEMS') + expect(result.errors![0]!.message).toContain( + 'no longer in the Code of Points', + ) + }) + + it('refuses too many ids, an unknown id and a badly formed id with BAD_USER_INPUT', async () => { + const many = Array.from( + { length: MAX_CALCULATOR_IDS + 1 }, + (_, i) => `UB-1.${i}-a`, + ) + expect(codeOf(await run(many))).toBe('BAD_USER_INPUT') + expect(codeOf(await run(['UB-99.999-a']))).toBe('BAD_USER_INPUT') + for (const bad of [ + '', + "UB-1.101-a'; DROP TABLE elements; --", + 'x'.repeat(100), + '%', + ]) { + expect(codeOf(await run([bad])), bad).toBe('BAD_USER_INPUT') + } + // Exactly 50 ids is fine. + const fifty = (await elements('FX', MAX_CALCULATOR_IDS)).map((r) => r.id) + expect(fifty.length).toBeGreaterThan(8) + expect((await run(fifty)).errors).toBeUndefined() + }) + + it('uses a fixed number of queries, not one per id', async () => { + const queries: string[] = [] + const { PrismaClient: Logging } = + await import('../generated/prisma/client.js') + const { PrismaPg } = await import('@prisma/adapter-pg') + const logging = new Logging({ + adapter: new PrismaPg({ connectionString: process.env['DATABASE_URL']! }), + log: [{ emit: 'event', level: 'query' }], + }) + logging.$on('query', (e) => queries.push(e.query)) + try { + const ids = (await elements('UB', 30)).map((r) => r.id) + await run(ids, logging as unknown as PrismaClient) + expect(queries.length).toBeLessThanOrEqual(2) + } finally { + await logging.$disconnect() + } + }) +}) diff --git a/backend/src/server/resolvers/calculator.ts b/backend/src/server/resolvers/calculator.ts new file mode 100644 index 0000000..668201e --- /dev/null +++ b/backend/src/server/resolvers/calculator.ts @@ -0,0 +1,64 @@ +// The calculator: the D-score of a list of items, for everyone. Nothing is saved. + +import { calculateDScore, type ScoredItem } from '../../dscore.js' +import { BadUserInputError, InvalidItemsError } from '../../errors.js' +import type { Context } from '../context.js' + +/** The most ids one call may have (also in docs/API.md). */ +export const MAX_CALCULATOR_IDS = 50 + +/** Ids look like "UB-2.305-a". This keeps odd text out of the queries and the messages. */ +const ID_PATTERN = /^[A-Za-z0-9.-]{1,40}$/ + +export const calculatorResolvers = { + Query: { + calculateDScore: async ( + _parent: unknown, + args: { itemIds: string[] }, + context: Context, + ): Promise => { + if (args.itemIds.length > MAX_CALCULATOR_IDS) { + throw new BadUserInputError( + `At most ${MAX_CALCULATOR_IDS} ids can be calculated at once.`, + ) + } + if (args.itemIds.some((id) => !ID_PATTERN.test(id))) { + throw new BadUserInputError('An item id is not valid.') + } + const ids = [...new Set(args.itemIds)] // the same id twice counts once + + const [elements, vaults] = await Promise.all([ + context.prisma.element.findMany({ where: { id: { in: ids } } }), + context.prisma.vault.findMany({ where: { id: { in: ids } } }), + ]) + + const found = new Set([...elements, ...vaults].map((item) => item.id)) + const unknown = ids.find((id) => !found.has(id)) + if (unknown !== undefined) { + throw new BadUserInputError(`There is no item with id ${unknown}.`) + } + const removed = [...elements, ...vaults].find((item) => item.removedAt) + if (removed) { + throw new InvalidItemsError( + `${removed.id} is no longer in the Code of Points. Remove it to get a score.`, + ) + } + + const items: ScoredItem[] = [ + ...elements.map((e) => ({ + id: e.id, + code: e.code, + value: e.value, + isVault: false, + })), + ...vaults.map((v) => ({ + id: v.id, + code: v.code, + value: v.dValue, + isVault: true, + })), + ] + return calculateDScore(items) + }, + }, +} diff --git a/backend/src/server/resolvers/index.ts b/backend/src/server/resolvers/index.ts index d8b6e47..9c0250c 100644 --- a/backend/src/server/resolvers/index.ts +++ b/backend/src/server/resolvers/index.ts @@ -1,3 +1,4 @@ +import { calculatorResolvers } from './calculator.js' import { catalogResolvers } from './catalog.js' import { userDataResolvers } from './userData.js' @@ -5,5 +6,9 @@ import { userDataResolvers } from './userData.js' export const resolvers = { ...catalogResolvers, ...userDataResolvers, - Query: { ...catalogResolvers.Query, ...userDataResolvers.Query }, + Query: { + ...catalogResolvers.Query, + ...calculatorResolvers.Query, + ...userDataResolvers.Query, + }, } diff --git a/backend/src/server/resolvers/userData.ts b/backend/src/server/resolvers/userData.ts index a11e20a..8c6f62e 100644 --- a/backend/src/server/resolvers/userData.ts +++ b/backend/src/server/resolvers/userData.ts @@ -25,7 +25,6 @@ export const userDataResolvers = { me: (): null => null, gymnast: loginRequired, routine: loginRequired, - calculateDScore: notImplemented('calculateDScore'), }, Mutation: { diff --git a/backend/src/server/server.test.ts b/backend/src/server/server.test.ts index 4cce540..8afd7cf 100644 --- a/backend/src/server/server.test.ts +++ b/backend/src/server/server.test.ts @@ -551,11 +551,6 @@ describe.runIf(hasDatabase)('the GraphQL server on the seeded catalog', () => { 'mutation { login(googleToken: "x") { token } }', 'NOT_IMPLEMENTED', ], - [ - 'calculateDScore', - '{ calculateDScore(itemIds: ["UB-1.101-a"]) }', - 'NOT_IMPLEMENTED', - ], ])('answers a clear error for %s', async (_name, query, code) => { const result = await run(query) expect(codeOf(result)).toBe(code) diff --git a/docs/AI_USAGE.md b/docs/AI_USAGE.md index 64ee17b..245fabe 100644 --- a/docs/AI_USAGE.md +++ b/docs/AI_USAGE.md @@ -264,6 +264,17 @@ `unwrapResolverError` did not recognise errors when two copies of the `graphql` package are loaded. Claude also started the server and tried queries, images, CORS and the error cases over HTTP. +## The calculator (`backend/src/dscore.ts`, `backend/src/server/resolvers/calculator.ts`) + +- **Tool:** Claude Code +- **What the team did:** Wrote issue #72 and the rule for the D-score earlier (the + eight highest values, each code once, the single vault on vault), and decided + that a removed item is refused by the calculator. +- **What Claude did:** Wrote the D-score function (the sum is made in tenths to + avoid rounding errors), the `calculateDScore` query with its errors, the + `INVALID_ITEMS` error class, the tests and the documentation, and checked + that the number of queries does not grow with the number of ids. + ## Database Infrastructure & Setup Completion (`backend/package.json`, `backend/.env.example`, `docs/VM_SETUP.md`, `docs/AI_USAGE.md`, `src/App.tsx`) diff --git a/docs/API.md b/docs/API.md index 60fd3db..7acf27b 100644 --- a/docs/API.md +++ b/docs/API.md @@ -79,7 +79,7 @@ When a new Code of Points drops an element or vault, it is marked as removed in - **`updatedAt` on a routine** changes when the routine is renamed and when its items change. The items are stored in a separate table, so `setRoutineItems` must update `updatedAt` explicitly, in the same transaction as the items. - **Names.** The name of a gymnast and the name of a routine are trimmed (spaces at the start and the end are removed) and must be 1–70 characters. A longer name, or a name with only spaces, throws `BAD_USER_INPUT`. The length is counted in characters and not in bytes, so a name with accents or emoji gets the same room as any other. 70 characters is the length that the UK Government Data Standards give for a full name in one field. Both limits are checked in the backend, and the database does not check them yet. `NAME_TAKEN` is checked on the trimmed name. - **Two tabs on the same routine.** `setRoutineItems` replaces the whole list, and the last save wins. It does not check if the routine was changed since the client loaded it, so a change made in another tab can be overwritten. This is a choice for the prototype. If it becomes a problem, add a `CONFLICT` error and let the client send the `updatedAt` it loaded. -- `calculateDScore(itemIds)` is for the calculator, which works without logging in. It calculates the D-score and saves nothing. +- `calculateDScore(itemIds)` is for the calculator, which works without logging in. It calculates the D-score and saves nothing. The rule is the same as for a saved routine (constraint 13 in [DATA_MODEL.md](DATA_MODEL.md)): the sum of the eight highest values, each code counted once, and on vault the D-value of the single vault. The sum is made in tenths (whole numbers), so there are no rounding errors. An empty list gives `0`, and the same id twice counts once. It throws `INVALID_ITEMS` when the items do not fit together (items from more than one apparatus, a vault with elements, or more than one vault) or when an item has been removed from the Code of Points, and `BAD_USER_INPUT` for more than 50 ids, an id that does not exist or an id with odd characters. The function is in `backend/src/dscore.ts` and `Routine.dScore` will use the same one. - The same element may appear more than once in a routine. Removed elements stay in routines and lists with `removed: true`, and make the routine's D-score null (see "Removed elements" in [DATA_MODEL.md](DATA_MODEL.md)). ## Search, filter, sort and pagination @@ -284,9 +284,9 @@ Until the login and the user data exist, every such operation answers a clear er | --------------------------------------------------------------------- | ----------------- | | `me` | `null` | | `gymnast`, `routine`, and every mutation that needs a logged-in coach | `UNAUTHENTICATED` | -| `login`, `calculateDScore` | `NOT_IMPLEMENTED` | +| `login` | `NOT_IMPLEMENTED` | -`NOT_IMPLEMENTED` is temporary and is not in the table of error codes. It goes away when the login and the calculator are built. A test checks that every mutation in the schema has a resolver, so a new mutation cannot be forgotten. +`NOT_IMPLEMENTED` is temporary and is not in the table of error codes. It goes away when the login and the user data are built. A test checks that every mutation in the schema has a resolver, so a new mutation cannot be forgotten. ## Tools @@ -407,6 +407,7 @@ Vitest runs the API tests against a test database, with queries sent to the serv - **Pagination:** every item is returned once when we follow the cursors to the end. This must also hold when many items have the same sort value, and for items without a name when sorting by name. `hasNextPage` is `false` on the last page. - **Filter and sort:** done over the whole catalog, not only the loaded page. - **Errors:** each code in the table is returned for the right case. A coach that asks for another coach's gymnast gets `NOT_FOUND`, and a request without a login gets `UNAUTHENTICATED` (but `me` gives `null`). +- **The calculator** (`backend/src/dscore.test.ts`, `backend/src/server/calculator.test.ts`): fewer than, exactly and more than eight elements, the same code twice, a vault, an empty list, sums that would give `0.30000000000000004` as floats; the query needs no login, gives the same score as the function for catalog items, refuses items from two apparatus, a vault with elements, two vaults and a removed item with `INVALID_ITEMS`, refuses 51 ids, an unknown id and an id with SQL in it with `BAD_USER_INPUT`, and uses a fixed number of queries. - **Wildcards and case:** a search for `%`, `_` or a word containing them does not match everything, and an uppercase search such as "SAL" finds the same items as "sal". - **Search text limits:** 81 characters or 9 words give `BAD_USER_INPUT`, and a text with no searchable words gives no results, while an empty text or only spaces gives all items. - **Deep pages:** the queries after a cursor contain no `OR` and no `OFFSET` above 0, so each one starts in the index right after the cursor.