diff --git a/.env.example b/.env.example index d17785f..cf00a63 100644 --- a/.env.example +++ b/.env.example @@ -21,3 +21,15 @@ JWT_SECRET=changeme-generate-a-real-random-secret-at-least-32-chars # over plain HTTP a Secure cookie is silently never sent back by the # browser, so login "succeeds" but every subsequent request 401s. # COOKIE_SECURE=false + +# Only needed to run the optional `tech-step-llm-worker` service — shared +# between it and "app" (docker-compose.yml). Generate your own the same +# way as JWT_SECRET above; leave both this and the service commented +# out/unset to run without it. +# INTERNAL_WORKER_SECRET=changeme-generate-a-real-random-secret-at-least-32-chars + +# Optional — cron expression (node-cron syntax) the worker wakes up on to +# run its audit/feedback-loop jobs. Default: weekly, Sunday 03:00 — a +# provisional floor, not a calibrated value (see +# services/tech-step-llm-worker/README.md). +# TECH_STEP_WORKER_CRON=0 3 * * 0 diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f17379c..919405d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -15,6 +15,10 @@ env: DATABASE_URL: "postgresql://ci:ci@localhost:5432/batchcooking_ci?schema=public" # Test-only secret, never used outside CI — real deployments must set their own. JWT_SECRET: "ci-only-secret-not-used-anywhere-else-32chars+" + # Same reasoning as JWT_SECRET above — lets tech-step-worker.routes.test.ts + # exercise the success path (matching secret), not just the "unset" + # rejection every environment that doesn't set this gets by default. + INTERNAL_WORKER_SECRET: "ci-only-worker-secret-not-used-anywhere-else-32chars+" jobs: # Four independent jobs, no needs: between them — each starts in parallel diff --git a/.gitignore b/.gitignore index 93666d2..3cff208 100644 --- a/.gitignore +++ b/.gitignore @@ -157,3 +157,7 @@ tmp-mockups/ # IA .claude/ + +# Cypress run artifacts — regenerated locally/in CI, never meant to be committed +apps/web/cypress/screenshots/ +apps/web/cypress/videos/ diff --git a/apps/api/.env.example b/apps/api/.env.example index 7dbf7af..9cd6e69 100644 --- a/apps/api/.env.example +++ b/apps/api/.env.example @@ -1,14 +1,20 @@ -NODE_ENV=development -PORT=3000 -# Match whatever you set in the root .env (POSTGRES_USER/PASSWORD/DB) — -# do not commit the real value. -DATABASE_URL="postgresql://changeme:changeme@localhost:5432/batchcooking?schema=public" - -# Required, no default on purpose — generate your own, e.g.: -# node -e "console.log(require('crypto').randomBytes(48).toString('hex'))" -JWT_SECRET=changeme-generate-a-real-random-secret-at-least-32-chars - -# Optional — defaults shown (see src/config/env.ts) -# JWT_EXPIRES_IN=7d -# AUTH_COOKIE_NAME=session -# CORS_ORIGIN=http://localhost:5173 +NODE_ENV=development +PORT=3000 +# Match whatever you set in the root .env (POSTGRES_USER/PASSWORD/DB) — +# do not commit the real value. +DATABASE_URL="postgresql://changeme:changeme@localhost:5432/batchcooking?schema=public" + +# Required, no default on purpose — generate your own, e.g.: +# node -e "console.log(require('crypto').randomBytes(48).toString('hex'))" +JWT_SECRET=changeme-generate-a-real-random-secret-at-least-32-chars + +# Optional — defaults shown (see src/config/env.ts) +# JWT_EXPIRES_IN=7d +# AUTH_COOKIE_NAME=session +# CORS_ORIGIN=http://localhost:5173 + +# Only needed if you're running services/tech-step-llm-worker locally — +# every /internal/tech-steps/* request is rejected outright while unset. +# Generate your own the same way as JWT_SECRET above; must match the +# worker's own INTERNAL_WORKER_SECRET. +# INTERNAL_WORKER_SECRET=changeme-generate-a-real-random-secret-at-least-32-chars diff --git a/apps/api/.env.test.example b/apps/api/.env.test.example index 2d467ad..975619d 100644 --- a/apps/api/.env.test.example +++ b/apps/api/.env.test.example @@ -13,3 +13,8 @@ DATABASE_URL="postgresql://changeme:changeme@localhost:5432/batchcooking_test?sc # Required, no default on purpose — generate your own, e.g.: # node -e "console.log(require('crypto').randomBytes(48).toString('hex'))" JWT_SECRET=changeme-generate-a-real-random-secret-at-least-32-chars + +# Optional — only needed to exercise tech-step-worker.routes.test.ts's +# success path (a request with a matching secret); every other test runs +# fine without it. Any value at least 32 chars works locally. +# INTERNAL_WORKER_SECRET=changeme-generate-a-real-random-secret-at-least-32-chars diff --git a/apps/api/prisma/migrations/20260822090000_tech_step_correction_and_suggestion/migration.sql b/apps/api/prisma/migrations/20260822090000_tech_step_correction_and_suggestion/migration.sql new file mode 100644 index 0000000..3210717 --- /dev/null +++ b/apps/api/prisma/migrations/20260822090000_tech_step_correction_and_suggestion/migration.sql @@ -0,0 +1,47 @@ +-- CreateTable +CREATE TABLE "step_tech_step_correction" ( + "id" SERIAL NOT NULL, + "step_id" INTEGER NOT NULL, + "corrector_id" INTEGER NOT NULL, + "start" INTEGER NOT NULL, + "end" INTEGER NOT NULL, + "previous_tech_step_id" INTEGER, + "corrected_tech_step_id" INTEGER, + "created_at" TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMP, + "consumed_at" TIMESTAMP(3), + + CONSTRAINT "step_tech_step_correction_pkey" PRIMARY KEY ("id") +); + +-- CreateTable +CREATE TABLE "tech_step_training_suggestion" ( + "id" SERIAL NOT NULL, + "tech_step_id" INTEGER NOT NULL, + "locale" TEXT NOT NULL, + "suggested_synonyms" TEXT[], + "suggested_utterances" TEXT[], + "source_type" TEXT NOT NULL, + "source_correction_id" INTEGER, + "status" TEXT NOT NULL DEFAULT 'pending', + "created_at" TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMP, + + CONSTRAINT "tech_step_training_suggestion_pkey" PRIMARY KEY ("id") +); + +-- AddForeignKey +ALTER TABLE "step_tech_step_correction" ADD CONSTRAINT "step_tech_step_correction_step_id_fkey" FOREIGN KEY ("step_id") REFERENCES "step"("id") ON DELETE CASCADE ON UPDATE CASCADE; + +-- AddForeignKey +ALTER TABLE "step_tech_step_correction" ADD CONSTRAINT "step_tech_step_correction_corrector_id_fkey" FOREIGN KEY ("corrector_id") REFERENCES "user_profiles"("id") ON DELETE CASCADE ON UPDATE CASCADE; + +-- AddForeignKey +ALTER TABLE "step_tech_step_correction" ADD CONSTRAINT "step_tech_step_correction_previous_tech_step_id_fkey" FOREIGN KEY ("previous_tech_step_id") REFERENCES "tech_step"("id") ON DELETE SET NULL ON UPDATE CASCADE; + +-- AddForeignKey +ALTER TABLE "step_tech_step_correction" ADD CONSTRAINT "step_tech_step_correction_corrected_tech_step_id_fkey" FOREIGN KEY ("corrected_tech_step_id") REFERENCES "tech_step"("id") ON DELETE SET NULL ON UPDATE CASCADE; + +-- AddForeignKey +ALTER TABLE "tech_step_training_suggestion" ADD CONSTRAINT "tech_step_training_suggestion_tech_step_id_fkey" FOREIGN KEY ("tech_step_id") REFERENCES "tech_step"("id") ON DELETE RESTRICT ON UPDATE CASCADE; + +-- AddForeignKey +ALTER TABLE "tech_step_training_suggestion" ADD CONSTRAINT "tech_step_training_suggestion_source_correction_id_fkey" FOREIGN KEY ("source_correction_id") REFERENCES "step_tech_step_correction"("id") ON DELETE SET NULL ON UPDATE CASCADE; diff --git a/apps/api/prisma/schema.prisma b/apps/api/prisma/schema.prisma index f1cf994..3f2c681 100644 --- a/apps/api/prisma/schema.prisma +++ b/apps/api/prisma/schema.prisma @@ -121,6 +121,10 @@ model UserProfile { /// list regardless of that real-world cardinality. administeredHouses House[] @relation("HouseAdmin") preferences UserPreference? + /// Tech-step corrections this profile has submitted (any profile that can + /// view a recipe may correct its tech-step matches, not just its author — + /// see `StepTechStepCorrection.correctorId`). + techStepCorrections StepTechStepCorrection[] @@map("user_profiles") } @@ -634,7 +638,16 @@ model TechStep { id Int @id @default(autoincrement()) key String @unique - steps StepTechStep[] + steps StepTechStep[] + /// Corrections where this technique was the *previous* (possibly wrong) + /// match — see `StepTechStepCorrection.previousTechStepId`. + correctionsAsPrevious StepTechStepCorrection[] @relation("PreviousTechStep") + /// Corrections where this technique was the *corrected* (user-asserted) + /// match — see `StepTechStepCorrection.correctedTechStepId`. + correctionsAsCorrected StepTechStepCorrection[] @relation("CorrectedTechStep") + /// Training-corpus suggestions targeting this technique — see + /// `TechStepTrainingSuggestion`. + trainingSuggestions TechStepTrainingSuggestion[] @@map("tech_step") } @@ -650,8 +663,11 @@ model Step { picture String? order Int - recipe Recipe @relation(fields: [recipeId], references: [id], onDelete: Cascade) - techSteps StepTechStep[] + recipe Recipe @relation(fields: [recipeId], references: [id], onDelete: Cascade) + techSteps StepTechStep[] + /// User-submitted corrections to this step's detected techniques — see + /// `StepTechStepCorrection`. + corrections StepTechStepCorrection[] @@map("step") } @@ -683,8 +699,8 @@ model Step { /// and recreates every `Step`/`StepTechStep`, never a partial patch) — /// graceful degradation, not a permanent gap. model StepTechStep { - stepId Int @map("step_id") - techStepId Int @map("tech_step_id") + stepId Int @map("step_id") + techStepId Int @map("tech_step_id") order Int start Int? end Int? @@ -697,3 +713,87 @@ model StepTechStep { @@id([stepId, order]) @@map("step_tech_step") } + +/// One user-submitted correction to a `Step`'s detected techniques — +/// captures ADD (a missing technique the classifier didn't find), +/// REMOVE (a wrong technique it did), or RELABEL (both) as a single shape: +/// `previousTechStepId` is the (possibly absent) match being corrected, +/// `correctedTechStepId` is what the user asserts instead (absent means +/// "no technique belongs here"). Both `null` at once is invalid (nothing +/// would have changed) — enforced service-side, not by the schema, same +/// posture as other cross-field invariants in this codebase (e.g. +/// `RecipeIngredientView`'s no-duplicate-ingredient check). +/// +/// `start`/`end` are the user's selected `[start, end)` span within +/// `Step.description` (`String.prototype.slice` convention, same as +/// `StepTechStep`) — what they highlighted before assigning a technique to +/// it, not necessarily identical to any existing `StepTechStep` span. +/// +/// Never edited/deleted once created (an audit trail of what was actually +/// submitted) — only `consumedAt` changes, stamped once +/// `services/tech-step-llm-worker` has turned this correction into a +/// `TechStepTrainingSuggestion` for a maintainer to review, so the same +/// correction isn't proposed twice on the next scheduled run. +model StepTechStepCorrection { + id Int @id @default(autoincrement()) + stepId Int @map("step_id") + /// Any profile that could *view* the recipe when they submitted this, not + /// necessarily its author — see `assertRecipeVisible`, + /// `recipe.service.ts`. + correctorId Int @map("corrector_id") + start Int + end Int + previousTechStepId Int? @map("previous_tech_step_id") + correctedTechStepId Int? @map("corrected_tech_step_id") + createdAt DateTime @default(now()) @map("created_at") + consumedAt DateTime? @map("consumed_at") + + step Step @relation(fields: [stepId], references: [id], onDelete: Cascade) + corrector UserProfile @relation(fields: [correctorId], references: [id], onDelete: Cascade) + previousTechStep TechStep? @relation("PreviousTechStep", fields: [previousTechStepId], references: [id], onDelete: SetNull) + correctedTechStep TechStep? @relation("CorrectedTechStep", fields: [correctedTechStepId], references: [id], onDelete: SetNull) + trainingSuggestions TechStepTrainingSuggestion[] + + @@map("step_tech_step_correction") +} + +/// A candidate addition to `TECH_STEP_TRAINING_DATA` +/// (`tech-step-training-data.ts`), proposed by `services/tech-step-llm-worker` +/// from one of two sources (`sourceType`): +/// +/// - `"correction"` — a user's `StepTechStepCorrection`, turned into +/// suggested synonyms/utterances by the worker's LLM +/// (`transform-corrections` job). +/// - `"llm_audit"` — a low-confidence NLP clause on an *existing* recipe the +/// worker periodically samples and re-judges with its LLM +/// (`audit-low-confidence` job); no `sourceCorrectionId` in this case. +/// +/// Deliberately never auto-applied to `tech-step-training-data.ts` — a +/// maintainer reviews `status: "pending"` rows (see +/// `list-pending-training-suggestions.ts`) and edits that file by hand, +/// same "generated suggestion, human-reviewed source of truth" split as a +/// linter's autofix vs. a human-authored diff. `retrain-tech-steps.ts` then +/// flips `status` to `"applied"`/`"rejected"` once a maintainer has acted on +/// a batch, so the same suggestion isn't reviewed twice. +/// +/// `suggestedSynonyms`/`suggestedUtterances` are native Postgres arrays +/// (`String[]`), not a join table — unlike this schema's other list-shaped +/// data (`RecipeDiet`, `UserProfileAllergy`...), these strings are free text +/// proposed once for a human to read, not ids referencing another catalog +/// table, so there's nothing for a join table to normalize against. +model TechStepTrainingSuggestion { + id Int @id @default(autoincrement()) + techStepId Int @map("tech_step_id") + locale String + suggestedSynonyms String[] @map("suggested_synonyms") + suggestedUtterances String[] @map("suggested_utterances") + sourceType String @map("source_type") + sourceCorrectionId Int? @map("source_correction_id") + status String @default("pending") + createdAt DateTime @default(now()) @map("created_at") + + techStep TechStep @relation(fields: [techStepId], references: [id]) + sourceCorrection StepTechStepCorrection? @relation(fields: [sourceCorrectionId], references: [id], onDelete: SetNull) + + @@map("tech_step_training_suggestion") +} diff --git a/apps/api/src/app.ts b/apps/api/src/app.ts index 4a5c86d..635cb55 100644 --- a/apps/api/src/app.ts +++ b/apps/api/src/app.ts @@ -7,6 +7,7 @@ import { errorLogger } from "./middlewares/error-logger.js"; import { requestLogger } from "./middlewares/request-logger.js"; import { authRouter } from "./modules/auth/auth.routes.js"; import { houseRouter } from "./modules/house/house.routes.js"; +import { techStepWorkerRouter } from "./modules/internal/tech-step-worker.routes.js"; import { planningRouter } from "./modules/planning/planning.routes.js"; import { preferencesRouter } from "./modules/preferences/preferences.routes.js"; import { profileRouter } from "./modules/profile/profile.routes.js"; @@ -38,6 +39,12 @@ export function createServer(): ExpressServer { server.mountRouter("/auth", authRouter); server.mountRouter("/house", houseRouter); + // Not user-facing — `services/tech-step-llm-worker` only, guarded by + // `requireInternalWorker` on every route within (see that router's own + // doc comment), never `requireAuth`. Mounted alongside the other routers + // rather than nested under one of them since it isn't scoped to a single + // recipe/step the way `recipeRouter`'s own correction routes are. + server.mountRouter("/internal/tech-steps", techStepWorkerRouter); server.mountRouter("/planning", planningRouter); server.mountRouter("/preferences", preferencesRouter); server.mountRouter("/profile", profileRouter); diff --git a/apps/api/src/config/env.ts b/apps/api/src/config/env.ts index 7f52193..47a86c4 100644 --- a/apps/api/src/config/env.ts +++ b/apps/api/src/config/env.ts @@ -60,6 +60,17 @@ const envSchema = z.object({ .string() .optional() .transform((value) => (value === undefined || value === "" ? undefined : value === "true")), + /** + * Shared secret `services/tech-step-llm-worker` sends as an + * `X-Internal-Worker-Secret` header on every call to `/internal/tech-steps/*` + * (`requireInternalWorker`, `middlewares/require-internal-worker.ts`). + * Optional with no default in the schema itself (unlike `JWT_SECRET`) so + * an environment that doesn't run the worker at all (e.g. this repo's + * existing test suite) never needs to set it — but `requireInternalWorker` + * itself rejects every request outright when it's unset, so the surface + * fails closed rather than open if a real deployment forgets to set it. + */ + INTERNAL_WORKER_SECRET: z.string().min(32).optional(), }); /** Parsed, validated environment — import this instead of reading `process.env` directly anywhere else. */ diff --git a/apps/api/src/lib/recipe-matching/tech-step-eval-dataset.ts b/apps/api/src/lib/recipe-matching/tech-step-eval-dataset.ts new file mode 100644 index 0000000..aa23638 --- /dev/null +++ b/apps/api/src/lib/recipe-matching/tech-step-eval-dataset.ts @@ -0,0 +1,265 @@ +/** + * Hand-labeled evaluation set for {@link techStepClassifier} — what + * `tech-step-eval.test.ts` runs the real classifier against to compute + * precision/recall/F1 (`tech-step-evaluator.ts`), the objective gate any + * future change to `tech-step-training-data.ts` must clear (see that + * module's own doc comment). + * + * Deliberately *not* reusing `TECH_STEP_TRAINING_DATA`'s own `utterances` + * verbatim — scoring the classifier against the exact sentences it was + * trained on would measure memorization, not generalization. Every + * description below is original phrasing; where a case still needs to name + * a technique's own verb to be labeled with confidence (most of them, see + * this file's own limits below), it's at least a different sentence shape + * than anything in the training corpus. + * + * `expectedKeys` is a multiset in reading order (see + * `TechStepEvalOutcome`'s doc comment in `tech-step-evaluator.ts` for why + * order isn't scored but repetition is) of `TechStep.key`s — resolved to + * real DB ids and back by `tech-step-eval-runner.ts`, this file only ever + * deals in stable keys so it doesn't need DB access to author or read. + * + * Known limit of this dataset: most cases anchor on a technique's own + * registered synonym (verb form), which `_classifyClause` always falls + * back to labeling correctly even when the intent classifier itself isn't + * confident (see `tech-step-matcher.ts`'s doc comment, point 3) — so this + * dataset mainly measures precision (wrong/duplicate matches, false + * positives from a synonym overlapping another technique's vocabulary) and + * breadth of coverage across all ~26 techniques, not the classifier's + * ability to recognize a technique described without ever naming it + * (`tech-step-matcher.test.ts` already covers a few of those specific, + * verified cases at the unit level — e.g. "jusqu'à ce que le beurre ait + * disparu dans la poêle" for `melt`). Extending this dataset with more + * paraphrase-only cases is valuable future work, but each one needs to be + * verified against a real trained classifier before being added (a wrong + * expected label here fails the regression gate for the wrong reason) — + * see this feature's plan document for the current gap in this session's + * ability to run the classifier locally. + */ + +export interface TechStepEvalCase { + description: string; + locale: "fr" | "en"; + expectedKeys: string[]; +} + +export const TECH_STEP_EVAL_DATASET: TechStepEvalCase[] = [ + // --- One straightforward case per technique (fr), covering all 26 --- + { + description: "Faites cuire les pâtes al dente dans une grande casserole d'eau bien salée.", + locale: "fr", + expectedKeys: ["cook"], + }, + { + description: "Faites bouillir l'eau dans une grande casserole avant d'y plonger les pâtes.", + locale: "fr", + expectedKeys: ["boil"], + }, + { + description: "Plongez les beignets dans l'huile très chaude pour les faire frire.", + locale: "fr", + expectedKeys: ["fry"], + }, + { + description: "Faites fondre le chocolat noir au bain-marie en remuant.", + locale: "fr", + expectedKeys: ["melt"], + }, + { + description: "Déglacez la casserole avec un trait de vinaigre balsamique.", + locale: "fr", + expectedKeys: ["deglaze"], + }, + { + description: "Laissez frémir la sauce tomate vingt minutes à couvert.", + locale: "fr", + expectedKeys: ["simmer"], + }, + { + description: "Faites rôtir la volaille entière sur la broche du four.", + locale: "fr", + expectedKeys: ["roast"], + }, + { + description: "Faites griller les brochettes de poulet quelques minutes de chaque côté.", + locale: "fr", + expectedKeys: ["grill"], + }, + { + description: "Faites sauter les champignons à feu vif dans une poêle très chaude.", + locale: "fr", + expectedKeys: ["panFry"], + }, + { + description: "Blanchissez les haricots verts trois minutes avant de les refroidir.", + locale: "fr", + expectedKeys: ["blanch"], + }, + { + description: "Laissez mariner les brochettes de poulet deux heures au frais.", + locale: "fr", + expectedKeys: ["marinate"], + }, + { + description: "Hachez grossièrement le persil frais avant de le parsemer.", + locale: "fr", + expectedKeys: ["chop"], + }, + { + description: "Épluchez les carottes avant de les couper en rondelles.", + locale: "fr", + expectedKeys: ["peel"], + }, + { + description: "Émincez finement l'échalote pour la vinaigrette.", + locale: "fr", + expectedKeys: ["mince"], + }, + { + description: "Mélangez la farine, le sucre et les œufs dans un grand saladier.", + locale: "fr", + expectedKeys: ["mix"], + }, + { + description: "Fouettez énergiquement la crème jusqu'à ce qu'elle épaississe.", + locale: "fr", + expectedKeys: ["whisk"], + }, + { + description: "Incorporez délicatement la farine tamisée à la préparation.", + locale: "fr", + expectedKeys: ["foldIn"], + }, + { + description: "Réservez la pâte au réfrigérateur pendant que vous préparez la garniture.", + locale: "fr", + expectedKeys: ["setAside"], + }, + { + description: "Assaisonnez le poisson avec du sel, du poivre et un filet de citron.", + locale: "fr", + expectedKeys: ["season"], + }, + { + description: "Égouttez soigneusement le riz dans une passoire fine.", + locale: "fr", + expectedKeys: ["drain"], + }, + { + description: + "Faites dorer les morceaux de veau sur toutes leurs faces avant de mouiller avec le bouillon.", + locale: "fr", + expectedKeys: ["brown"], + }, + { + description: "Laissez reposer la viande dix minutes avant de la trancher.", + locale: "fr", + expectedKeys: ["rest"], + }, + { + description: "Préchauffez le four à 200 degrés avant d'y glisser le gratin.", + locale: "fr", + expectedKeys: ["preheat"], + }, + { + description: "Enfournez la tarte pendant trente-cinq minutes jusqu'à ce qu'elle soit dorée.", + locale: "fr", + expectedKeys: ["bake"], + }, + { + description: "Dressez harmonieusement les légumes autour de la pièce de viande.", + locale: "fr", + expectedKeys: ["plate"], + }, + { + description: "Nappez le fond du moule d'une fine couche de caramel.", + locale: "fr", + expectedKeys: ["coat"], + }, + + // --- English coverage (same technique verbs, distinct sentences) --- + { + description: "Simmer the stock gently for forty minutes, skimming occasionally.", + locale: "en", + expectedKeys: ["simmer"], + }, + { + description: "Peel the potatoes and rinse them under cold water.", + locale: "en", + expectedKeys: ["peel"], + }, + { + description: "Whisk the eggs with a pinch of salt until frothy.", + locale: "en", + expectedKeys: ["whisk"], + }, + { + description: "Season the soup generously with black pepper before serving.", + locale: "en", + expectedKeys: ["season"], + }, + { + description: "Make sure the chicken is cooked through before serving.", + locale: "en", + expectedKeys: ["cook"], + }, + + // --- Multi-technique sentences, in reading order --- + { + description: "Préchauffez le four, puis faites rôtir le poulet pendant une heure.", + locale: "fr", + expectedKeys: ["preheat", "roast"], + }, + { + description: "Faites revenir les oignons, puis déglacez la poêle avec du vin blanc.", + locale: "fr", + expectedKeys: ["brown", "deglaze"], + }, + { + description: + "Faites cuire les légumes à la vapeur, puis assaisonnez-les avec des herbes fraîches.", + locale: "fr", + expectedKeys: ["cook", "season"], + }, + { + description: + "Émincez l'oignon, faites-le suer, puis mouillez avec le bouillon et laissez mijoter.", + locale: "fr", + expectedKeys: ["mince", "simmer"], + }, + + // --- No technique mentioned at all --- + { + description: "Répartissez les convives autour de la table avant de commencer le repas.", + locale: "fr", + expectedKeys: [], + }, + { + description: "Rangez les couverts propres dans le tiroir de la cuisine.", + locale: "fr", + expectedKeys: [], + }, + { + description: "Take the plates and glasses out of the cupboard.", + locale: "en", + expectedKeys: [], + }, + + // --- Documented false-positive traps, re-verified with fresh wording --- + // `brown`'s EN synonyms are verb forms only ("browned"/"browning"), not + // bare "brown" — precisely so this doesn't false-positive (see that + // entry's own comment in tech-step-training-data.ts). + { + description: "This recipe calls for two tablespoons of brown sugar.", + locale: "en", + expectedKeys: [], + }, + // `rest`'s EN synonyms are anchored phrases ("let it rest"/"resting + // for"...), not bare "rest" — so a sentence using the word in its + // "remainder" sense must not anchor `rest` at all. + { + description: "There is no time to rest before the guests arrive.", + locale: "en", + expectedKeys: [], + }, +]; diff --git a/apps/api/src/lib/recipe-matching/tech-step-eval-runner.ts b/apps/api/src/lib/recipe-matching/tech-step-eval-runner.ts new file mode 100644 index 0000000..d53df1e --- /dev/null +++ b/apps/api/src/lib/recipe-matching/tech-step-eval-runner.ts @@ -0,0 +1,59 @@ +import { prisma } from "../../db/prisma.js"; +import { TECH_STEP_EVAL_DATASET } from "./tech-step-eval-dataset.js"; +import { + computeTechStepMetrics, + type TechStepEvalOutcome, + type TechStepEvalResult, +} from "./tech-step-evaluator.js"; +import { techStepClassifier } from "./tech-step-matcher.js"; + +/** + * Provisional floor (not a target) both `runTechStepEvalSuite`'s + * consumers gate on — see `test/recipe-matching/tech-step-eval.test.ts`'s + * own doc comment for the full reasoning behind this specific value and + * when to tighten it. Exported from here (not defined separately in each + * consumer) so the CI regression gate and `scripts/retrain-tech-steps.ts`'s + * pre-backfill gate can never silently drift to different thresholds. + */ +export const MIN_OVERALL_F1 = 0.8; + +/** + * Runs {@link TECH_STEP_EVAL_DATASET} against the real, currently-trained + * `techStepClassifier` and returns the aggregate/per-technique metrics + * (`computeTechStepMetrics`, `tech-step-evaluator.ts`) — the one place this + * DB-touching "resolve ids to keys, then score" logic lives, shared by + * `test/recipe-matching/tech-step-eval.test.ts` (this feature's CI + * regression gate) and `scripts/retrain-tech-steps.ts` (the same gate, run + * by a maintainer before applying a corpus change). Kept out of + * `tech-step-evaluator.ts` itself, which is deliberately pure/DB-free (see + * that module's own doc comment) so its scoring logic stays unit-testable + * without a database. + */ +export async function runTechStepEvalSuite(): Promise { + const techSteps = await prisma.techStep.findMany({ select: { id: true, key: true } }); + const keyById = new Map(techSteps.map((techStep) => [techStep.id, techStep.key])); + + const outcomes: TechStepEvalOutcome[] = []; + for (const evalCase of TECH_STEP_EVAL_DATASET) { + const techStepIds = await techStepClassifier.matchTechSteps( + evalCase.description, + evalCase.locale, + ); + const actualKeys = techStepIds.map((id) => { + const key = keyById.get(id); + // A `techStepId` the classifier resolved that isn't in the seeded + // catalog would be a bug in the classifier or the seed data, not + // this dataset — fail loudly rather than silently dropping it (see + // `_train`'s own comment in `tech-step-matcher.ts` on the + // equivalent, deliberately silent `undefined` case it has to + // tolerate for a different reason). + if (key === undefined) { + throw new Error(`Unknown TechStep id ${id} returned for "${evalCase.description}"`); + } + return key; + }); + outcomes.push({ expectedKeys: evalCase.expectedKeys, actualKeys }); + } + + return computeTechStepMetrics(outcomes); +} diff --git a/apps/api/src/lib/recipe-matching/tech-step-evaluator.ts b/apps/api/src/lib/recipe-matching/tech-step-evaluator.ts new file mode 100644 index 0000000..6158189 --- /dev/null +++ b/apps/api/src/lib/recipe-matching/tech-step-evaluator.ts @@ -0,0 +1,134 @@ +/** + * Precision/recall/F1 for {@link techStepClassifier}'s output against a + * hand-labeled evaluation set (`tech-step-eval-dataset.ts`) — the objective + * counterpart to the "inspected by eye" verdict every corpus change used to + * get before this module existed. Every future edit to + * `tech-step-training-data.ts` (including the LLM-assisted suggestions the + * worker in `services/tech-step-llm-worker` proposes) is expected to run + * through `tech-step-eval.test.ts`'s regression gate, which calls + * {@link computeTechStepMetrics} — a corpus change that raises recall on one + * technique but silently tanks another's precision should fail loudly here, + * not get merged on the strength of a few manually-checked examples. + * + * Pure (no DB/model access) so it's unit-testable on its own — same + * convention as `tech-step-matcher.ts`'s own pure helpers (`normalizeText`, + * `splitIntoClauses`): this module only ever receives already-resolved + * `TechStep.key` strings, never DB ids or a live classifier instance, so it + * has nothing to mock to test. + */ + +/** True/false-positive/negative counts for one technique (or the aggregate across all of them), plus the precision/recall/F1 derived from them. */ +export interface TechStepMetrics { + truePositives: number; + falsePositives: number; + falseNegatives: number; + precision: number; + recall: number; + f1: number; +} + +/** + * One evaluation case's outcome — what {@link TechStepEvalCase.expectedKeys} + * said should be found, against what the classifier actually returned for + * that case (already mapped from `TechStepMatch.techStepId` back to + * `TechStep.key`, see `tech-step-eval.test.ts`). + * + * Both lists are *multisets*, not sets — a description that names the same + * technique twice (rare, but not impossible: "faire cuire, puis... remettre + * à cuire") is expected to produce two matches, and comparing as plain sets + * would silently treat a classifier that only found one of them as a + * perfect match. + */ +export interface TechStepEvalOutcome { + expectedKeys: string[]; + actualKeys: string[]; +} + +/** {@link computeTechStepMetrics}'s result — the aggregate across every case, plus a breakdown per technique so a regression hiding behind a healthy overall F1 (one technique's recall collapsing, offset by another's improving) is still visible. */ +export interface TechStepEvalResult { + overall: TechStepMetrics; + byKey: Record; +} + +interface RawCounts { + tp: number; + fp: number; + fn: number; +} + +function emptyCounts(): RawCounts { + return { tp: 0, fp: 0, fn: 0 }; +} + +/** Counts occurrences of each key in a multiset, e.g. `["cook", "cook", "bake"]` -> `{cook: 2, bake: 1}`. */ +function countByKey(keys: string[]): Map { + const counts = new Map(); + for (const key of keys) { + counts.set(key, (counts.get(key) ?? 0) + 1); + } + return counts; +} + +/** + * Standard vacuous-truth convention for the `0/0` cases: precision defaults + * to `1` when nothing was predicted for a key (`tp + fp === 0` — no false + * accusation to be precise about), recall defaults to `1` when nothing was + * expected (`tp + fn === 0` — nothing to have missed). Neither inflates F1 + * on its own: a technique the classifier fully misses still has `recall = + * 0` (there *were* expected occurrences, just none matched), which is what + * pulls F1 down to `0` for that case regardless of precision's vacuous `1`. + */ +function toMetrics(counts: RawCounts): TechStepMetrics { + const { tp, fp, fn } = counts; + const precision = tp + fp === 0 ? 1 : tp / (tp + fp); + const recall = tp + fn === 0 ? 1 : tp / (tp + fn); + const f1 = precision + recall === 0 ? 0 : (2 * precision * recall) / (precision + recall); + return { truePositives: tp, falsePositives: fp, falseNegatives: fn, precision, recall, f1 }; +} + +/** + * Aggregates every {@link TechStepEvalOutcome} into one overall + * precision/recall/F1 plus a per-technique breakdown. + * + * Counted key by key, multiset-style, per outcome: for a given technique, + * `min(expectedCount, actualCount)` true positives, any actual occurrences + * beyond that are false positives, any expected occurrences short of that + * are false negatives — generalizes the usual set-based TP/FP/FN definition + * to handle a technique mentioned (or matched) more than once in the same + * step without over- or under-counting it. + */ +export function computeTechStepMetrics(outcomes: TechStepEvalOutcome[]): TechStepEvalResult { + const overallCounts = emptyCounts(); + const countsByKey = new Map(); + + for (const outcome of outcomes) { + const expectedCounts = countByKey(outcome.expectedKeys); + const actualCounts = countByKey(outcome.actualKeys); + const allKeys = new Set([...expectedCounts.keys(), ...actualCounts.keys()]); + + for (const key of allKeys) { + const expected = expectedCounts.get(key) ?? 0; + const actual = actualCounts.get(key) ?? 0; + const tp = Math.min(expected, actual); + const fp = Math.max(0, actual - expected); + const fn = Math.max(0, expected - actual); + + overallCounts.tp += tp; + overallCounts.fp += fp; + overallCounts.fn += fn; + + const keyCounts = countsByKey.get(key) ?? emptyCounts(); + keyCounts.tp += tp; + keyCounts.fp += fp; + keyCounts.fn += fn; + countsByKey.set(key, keyCounts); + } + } + + const byKey: Record = {}; + for (const [key, counts] of countsByKey) { + byKey[key] = toMetrics(counts); + } + + return { overall: toMetrics(overallCounts), byKey }; +} diff --git a/apps/api/src/lib/recipe-matching/tech-step-matcher.ts b/apps/api/src/lib/recipe-matching/tech-step-matcher.ts index 9f7b580..9bfff45 100644 --- a/apps/api/src/lib/recipe-matching/tech-step-matcher.ts +++ b/apps/api/src/lib/recipe-matching/tech-step-matcher.ts @@ -252,7 +252,32 @@ export function splitIntoClauses( * — `0.75` sits comfortably above the noise floor and below every genuine * match seen so far. */ -const CONFIDENCE_THRESHOLD = 0.75; +export const CONFIDENCE_THRESHOLD = 0.75; + +/** + * One clause's full classification detail — the finer-grained sibling of + * {@link TechStepMatch}, exposing the raw intent/score + * `TechStepClassifierService`'s private `_classifyClause` normally + * collapses into a single accepted-or-fallback verdict. Nothing on the + * interactive save/read path needs this (that's exactly what + * `_classifyClause`'s threshold + fallback logic is for) — it exists for + * `services/tech-step-llm-worker`'s "audit low-confidence clauses" job + * (`modules/internal/tech-step-worker.service.ts`'s `getAuditBatch`), which + * needs to see *which* clauses the classifier itself wasn't sure about, not + * just its final best-effort verdict. + */ +export interface TechStepClauseClassification { + /** The clause's own text (`description.slice(start, end)`, trimmed). */ + clauseText: string; + start: number; + end: number; + /** The clause's NER anchor's own implied technique `uid`, if it had one — same as `TechStepClause.anchor.uid`. */ + anchorUid: string | null; + /** The intent classifier's own top guess for this clause, whatever its score — `null` only when it returned node-nlp's `"None"` sentinel. Unlike {@link TechStepMatch}, never silently replaced by the anchor's uid — the whole point of this type is to expose the classifier's raw opinion, confident or not. */ + intentUid: string | null; + /** The intent classifier's own confidence for `intentUid` — `0` when `intentUid` is `null` (nothing to have a score about). */ + score: number; +} /** * Trains and owns the `node-nlp` model behind {@link matchTechStepSpans} — @@ -388,6 +413,66 @@ export class TechStepClassifierService { } } + /** + * Splits `description` into clauses exactly like {@link matchTechStepSpans} + * does, but returns each clause's *raw* classification detail + * ({@link TechStepClauseClassification}) instead of the threshold-applied, + * anchor-fallback-resolved `TechStepMatch` — see that type's doc comment + * for why/who needs this. Deliberately a separate traversal rather than a + * shared refactor with `matchTechStepSpans`/`_classifyClause`: this method + * exists purely to add a new, additive read path without risking a + * behavior change to the two already-relied-on methods above. + */ + public async classifyClauses( + description: string, + locale: string, + ): Promise { + try { + await this._ensureTrained(); + if (description.trim().length === 0) return []; + + const nerResult = await this._manager.process(locale, description); + const candidates: TechniqueCandidate[] = nerResult.entities + .filter((entity) => entity.type === "enum") + .map((entity) => ({ + uid: entity.entity, + start: entity.start, + end: entity.end + 1, + })); + + const clauses = splitIntoClauses(description, candidates); + const results: TechStepClauseClassification[] = []; + for (const clause of clauses) { + const clauseText = description.slice(clause.start, clause.end).trim(); + const anchorUid = clause.anchor?.uid ?? null; + if (clauseText.length === 0) { + results.push({ + clauseText, + start: clause.start, + end: clause.end, + anchorUid, + intentUid: null, + score: 0, + }); + continue; + } + const result = await this._manager.process(locale, clauseText); + const intentUid = result.intent !== "None" ? result.intent : null; + results.push({ + clauseText, + start: clause.start, + end: clause.end, + anchorUid, + intentUid, + score: intentUid === null ? 0 : result.score, + }); + } + return results; + } catch (err) { + throw err; // see matchTechStepSpans()'s catch comment above + } + } + /** * Convenience wrapper around {@link matchTechStepSpans} for callers that * only care about *which* techniques matched, not where — e.g. diff --git a/apps/api/src/middlewares/require-internal-worker.ts b/apps/api/src/middlewares/require-internal-worker.ts new file mode 100644 index 0000000..f74535e --- /dev/null +++ b/apps/api/src/middlewares/require-internal-worker.ts @@ -0,0 +1,50 @@ +import { timingSafeEqual } from "node:crypto"; +import { HttpError } from "@batch-cooking/error-tools"; +import { ErrorCode } from "@batch-cooking/shared"; +import type { NextFunction, Request, Response } from "express"; +import { env } from "../config/env.js"; + +/** Header `services/tech-step-llm-worker` sends its shared secret on. Not `Authorization`/a bearer scheme — this isn't a user session, just one internal caller authenticating to another, same "one flat shared secret" shape as e.g. a webhook signing header. */ +const INTERNAL_WORKER_SECRET_HEADER = "x-internal-worker-secret"; + +/** + * Express middleware guarding `/internal/tech-steps/*` — the surface + * `services/tech-step-llm-worker` (a process outside this monorepo, no + * Prisma access of its own, see that service's own README) reads + * low-confidence NLP clauses and pending `StepTechStepCorrection`s from, + * and posts `TechStepTrainingSuggestion`s back to. Never reachable by an + * end user's session cookie — deliberately a *different* auth mechanism + * than {@link requireAuth} (`require-auth.ts`), not layered on top of it, + * since the worker has no `UserProfile`/session of its own to authenticate + * as. + * + * Fails closed: an unset `INTERNAL_WORKER_SECRET` (the default in any + * environment that doesn't run the worker, see `config/env.ts`) rejects + * every request rather than leaving the surface open, same posture as a + * misconfigured `JWT_SECRET` would if it had a working fallback. + * + * @throws {HttpError} `401 NOT_AUTHENTICATED` if the header is missing, + * wrong, or the server has no secret configured at all — never + * distinguishes the reason, same posture as {@link requireAuth}. + */ +export function requireInternalWorker(req: Request, _res: Response, next: NextFunction): void { + const provided = req.header(INTERNAL_WORKER_SECRET_HEADER); + if (env.INTERNAL_WORKER_SECRET === undefined || provided === undefined) { + next(new HttpError(401, ErrorCode.NOT_AUTHENTICATED, "Not authenticated")); + return; + } + + // `timingSafeEqual` throws on mismatched buffer lengths rather than + // returning `false` — checked separately first. A length mismatch alone + // already means "not equal", so this loses no timing-attack protection + // (an attacker learns nothing beyond what a differing length itself + // already reveals, no different from `!==` on the common case where the + // secret's real length isn't a secret worth protecting). + const expected = Buffer.from(env.INTERNAL_WORKER_SECRET); + const actual = Buffer.from(provided); + if (expected.length !== actual.length || !timingSafeEqual(expected, actual)) { + next(new HttpError(401, ErrorCode.NOT_AUTHENTICATED, "Not authenticated")); + return; + } + next(); +} diff --git a/apps/api/src/modules/internal/tech-step-worker.routes.ts b/apps/api/src/modules/internal/tech-step-worker.routes.ts new file mode 100644 index 0000000..748cd9a --- /dev/null +++ b/apps/api/src/modules/internal/tech-step-worker.routes.ts @@ -0,0 +1,49 @@ +import { wrapAsyncHandler } from "@batch-cooking/express-tools"; +import { + auditBatchQuerySchema, + submitTrainingSuggestionsSchema, + workerBatchQuerySchema, +} from "@batch-cooking/shared"; +import { Router } from "express"; +import { requireInternalWorker } from "../../middlewares/require-internal-worker.js"; +import { + getAuditBatch, + getPendingCorrections, + submitTrainingSuggestions, +} from "./tech-step-worker.service.js"; + +/** + * Router mounted at `/internal/tech-steps` in app.ts — every route requires + * {@link requireInternalWorker}, never {@link requireAuth} + * (`middlewares/require-auth.ts`): this is `services/tech-step-llm-worker` + * authenticating as itself, not a user session. See that middleware's own + * doc comment for why the two are deliberately separate mechanisms. + */ +export const techStepWorkerRouter = Router(); + +techStepWorkerRouter.get( + "/audit-batch", + requireInternalWorker, + wrapAsyncHandler(async (req, res) => { + const input = auditBatchQuerySchema.parse(req.query); + res.status(200).json(await getAuditBatch(input.locale, input.limit)); + }), +); + +techStepWorkerRouter.get( + "/pending-corrections", + requireInternalWorker, + wrapAsyncHandler(async (req, res) => { + const input = workerBatchQuerySchema.parse(req.query); + res.status(200).json(await getPendingCorrections(input.limit)); + }), +); + +techStepWorkerRouter.post( + "/training-suggestions", + requireInternalWorker, + wrapAsyncHandler(async (req, res) => { + const input = submitTrainingSuggestionsSchema.parse(req.body); + res.status(201).json(await submitTrainingSuggestions(input)); + }), +); diff --git a/apps/api/src/modules/internal/tech-step-worker.service.ts b/apps/api/src/modules/internal/tech-step-worker.service.ts new file mode 100644 index 0000000..607c0f1 --- /dev/null +++ b/apps/api/src/modules/internal/tech-step-worker.service.ts @@ -0,0 +1,201 @@ +import { HttpError } from "@batch-cooking/error-tools"; +import { + ErrorCode, + type PendingTechStepCorrectionView, + type SubmitTrainingSuggestionsInput, + type TechStepAuditClauseView, +} from "@batch-cooking/shared"; +import { prisma } from "../../db/prisma.js"; +import { + CONFIDENCE_THRESHOLD, + techStepClassifier, +} from "../../lib/recipe-matching/tech-step-matcher.js"; + +/** + * Read/write surface `services/tech-step-llm-worker` calls through + * `/internal/tech-steps/*` (`tech-step-worker.routes.ts`, guarded by + * `requireInternalWorker`) — the worker has no Prisma client or database + * credentials of its own (see that service's own README), so every + * corrections/audit-sample read and every suggestion write goes through + * here rather than the worker touching this schema directly. Keeps + * `apps/api` the single owner of the schema/migrations, and keeps the + * worker a pure "read some text, run inference, post a suggestion" process + * with nothing to keep in sync if the schema changes shape. + */ + +/** + * How many of the most recently created `Step`s {@link getAuditBatch} scans + * per call before filtering down to low-confidence clauses — a fixed + * recency-biased sample, not every `Step` in the database, to keep this + * endpoint's cost bounded regardless of how large the recipe catalog gets. + * Recently-added steps are also the steps most likely to still use + * vocabulary the training corpus hasn't caught up with yet, which is + * exactly what this audit is for. A smarter sampling strategy (e.g. + * weighted by how often a recipe is actually viewed/planned) is future + * work, not needed for this feature's first version. + */ +const AUDIT_SAMPLE_SIZE = 200; + +/** + * Every low-confidence clause found across a recency-biased sample of + * existing `Step`s (see {@link AUDIT_SAMPLE_SIZE}), for + * `services/tech-step-llm-worker`'s `audit-low-confidence` job to get a + * second opinion on. "Low-confidence" mirrors exactly what + * `TechStepClassifierService._classifyClause` itself distrusts (a clause + * with an NER anchor but a classifier score under + * {@link CONFIDENCE_THRESHOLD}) — the same clauses that pipeline already + * has to fall back to keyword-anchor guessing for, not an arbitrary + * separate cutoff. + */ +export async function getAuditBatch( + locale: string, + limit: number, +): Promise { + try { + const steps = await prisma.step.findMany({ + orderBy: { id: "desc" }, + take: AUDIT_SAMPLE_SIZE, + select: { id: true, recipeId: true, description: true }, + }); + + const results: TechStepAuditClauseView[] = []; + for (const step of steps) { + if (results.length >= limit) break; + const clauses = await techStepClassifier.classifyClauses(step.description, locale); + for (const clause of clauses) { + if (results.length >= limit) break; + const isLowConfidence = clause.anchorUid !== null && clause.score < CONFIDENCE_THRESHOLD; + if (!isLowConfidence) continue; + results.push({ + stepId: step.id, + recipeId: step.recipeId, + clauseText: clause.clauseText, + anchorKey: clause.anchorUid, + intentKey: clause.intentUid, + score: clause.score, + locale, + }); + } + } + return results; + } catch (err) { + throw err; // see recipe.service.ts's equivalent catch comment + } +} + +/** + * Every `StepTechStepCorrection` not yet turned into a + * `TechStepTrainingSuggestion` (`consumedAt IS NULL`), oldest first — a + * FIFO queue the worker's `transform-corrections` job drains, `limit` at a + * time. + * + * `correctedTechStepId IS NOT NULL` on top of `consumedAt IS NULL`: a + * correction that *removes* a match ("no technique belongs here", + * `correctedTechStepId: null` — see `StepTechStepCorrection`'s schema doc + * comment) has no technique to propose new positive training data *for*. + * Surfacing it here would leave it permanently unconsumable (the worker + * has nothing to submit a suggestion for, so it would never stamp + * `consumedAt`, and it would keep re-appearing in every future batch + * forever) — excluded at the source instead, not filtered/skipped + * downstream by the worker. + */ +export async function getPendingCorrections( + limit: number, +): Promise { + try { + const corrections = await prisma.stepTechStepCorrection.findMany({ + where: { consumedAt: null, correctedTechStepId: { not: null } }, + orderBy: { createdAt: "asc" }, + take: limit, + include: { + step: { select: { id: true, recipeId: true, description: true } }, + previousTechStep: { select: { key: true } }, + correctedTechStep: { select: { key: true } }, + }, + }); + + return corrections.map((correction) => ({ + id: correction.id, + stepId: correction.step.id, + recipeId: correction.step.recipeId, + clauseText: correction.step.description.slice(correction.start, correction.end), + start: correction.start, + end: correction.end, + previousTechStepKey: correction.previousTechStep?.key ?? null, + correctedTechStepKey: correction.correctedTechStep?.key ?? null, + })); + } catch (err) { + throw err; // see recipe.service.ts's equivalent catch comment + } +} + +/** + * Persists a batch of `TechStepTrainingSuggestion`s and, for every + * suggestion sourced from a correction, stamps that correction's + * `consumedAt` in the same transaction — so a worker run that crashes + * partway through never leaves a correction consumed with no matching + * suggestion, or a suggestion created against a correction still (wrongly) + * eligible to be picked up again by the next run. + * + * @throws {HttpError} `404 TECH_STEP_NOT_FOUND` if any `techStepKey` in the + * batch doesn't match a reference `TechStep` — rejects the *whole* batch + * rather than skipping the bad entries, on the theory that a worker + * sending an unknown key is more likely a version-skew bug (its own + * taxonomy copy, `services/tech-step-llm-worker/src/tech-step-taxonomy.ts`, + * drifting from this API's `TechStep` catalog) than a one-off it should + * silently tolerate. + */ +export async function submitTrainingSuggestions( + input: SubmitTrainingSuggestionsInput, +): Promise<{ created: number }> { + try { + const techStepKeys = [ + ...new Set(input.suggestions.map((suggestion) => suggestion.techStepKey)), + ]; + const techSteps = await prisma.techStep.findMany({ + where: { key: { in: techStepKeys } }, + select: { id: true, key: true }, + }); + const techStepIdByKey = new Map(techSteps.map((techStep) => [techStep.key, techStep.id])); + const missingKeys = techStepKeys.filter((key) => !techStepIdByKey.has(key)); + if (missingKeys.length > 0) { + throw new HttpError( + 404, + ErrorCode.TECH_STEP_NOT_FOUND, + `Unknown techStepKey(s): ${missingKeys.join(", ")}`, + ); + } + + await prisma.$transaction(async (tx) => { + for (const suggestion of input.suggestions) { + // Non-null by construction — every key in `input.suggestions` was + // just confirmed present in `techStepIdByKey` above (the `missingKeys` + // check would have thrown otherwise). + const techStepId = techStepIdByKey.get(suggestion.techStepKey); + if (techStepId === undefined) continue; + + await tx.techStepTrainingSuggestion.create({ + data: { + techStepId, + locale: suggestion.locale, + suggestedSynonyms: suggestion.suggestedSynonyms, + suggestedUtterances: suggestion.suggestedUtterances, + sourceType: suggestion.sourceType, + sourceCorrectionId: suggestion.sourceCorrectionId ?? null, + }, + }); + + if (suggestion.sourceCorrectionId !== null && suggestion.sourceCorrectionId !== undefined) { + await tx.stepTechStepCorrection.update({ + where: { id: suggestion.sourceCorrectionId }, + data: { consumedAt: new Date() }, + }); + } + } + }); + + return { created: input.suggestions.length }; + } catch (err) { + throw err; // see recipe.service.ts's equivalent catch comment + } +} diff --git a/apps/api/src/modules/recipe/recipe-tech-step-correction.service.ts b/apps/api/src/modules/recipe/recipe-tech-step-correction.service.ts new file mode 100644 index 0000000..9830681 --- /dev/null +++ b/apps/api/src/modules/recipe/recipe-tech-step-correction.service.ts @@ -0,0 +1,180 @@ +import { HttpError } from "@batch-cooking/error-tools"; +import { + ErrorCode, + type StepTechStepCorrectionView, + type SubmitTechStepCorrectionInput, +} from "@batch-cooking/shared"; +import type { Prisma } from "@prisma/client"; +import { prisma } from "../../db/prisma.js"; +import { assertRecipeVisible } from "./recipe.service.js"; + +/** + * User-submitted corrections to a step's detected techniques + * (`StepTechStepCorrection` in schema.prisma) — kept in its own module + * rather than folded into `recipe.service.ts`, same "one file per concern" + * split that file itself follows for `tech-step-matcher.ts`. Deliberately + * open to *any* viewer who can see the recipe, not just its author (unlike + * every write path in `recipe.service.ts`, which uses `assertIsAuthor`) — + * correcting a mislabeled technique isn't editing the recipe's own + * content, and restricting it to authors would starve the training-data + * feedback loop (`services/tech-step-llm-worker`) of the volume it needs. + */ + +type CorrectionWithTechSteps = Prisma.StepTechStepCorrectionGetPayload<{ + include: { previousTechStep: true; correctedTechStep: true }; +}>; + +const correctionInclude = { + previousTechStep: true, + correctedTechStep: true, +} satisfies Prisma.StepTechStepCorrectionInclude; + +/** + * Loads `stepId`'s current `description` length (the only thing a + * correction needs from the step itself), or throws — `404 STEP_NOT_FOUND` + * if no such step exists, or if it exists but doesn't belong to `recipeId` + * (the route's own `:id`/`:stepId` nesting is meaningless otherwise — a + * request naming a real step under the wrong recipe should look identical + * to naming one that doesn't exist, same "don't leak which part was wrong" + * posture `assertRecipeVisible` already has for visibility). Otherwise + * whatever {@link assertRecipeVisible} throws (`404 RECIPE_NOT_FOUND`, + * never `403`) if the recipe exists but isn't visible to the viewer. + */ +async function loadVisibleStepOrThrow( + recipeId: number, + stepId: number, + viewerId: number, + viewerHouseId: number | null, +): Promise<{ id: number; descriptionLength: number }> { + try { + const step = await prisma.step.findUnique({ + where: { id: stepId }, + select: { id: true, recipeId: true, description: true }, + }); + if (!step || step.recipeId !== recipeId) { + throw new HttpError(404, ErrorCode.STEP_NOT_FOUND, `Step ${stepId} not found`); + } + await assertRecipeVisible(step.recipeId, viewerId, viewerHouseId); + return { id: step.id, descriptionLength: step.description.length }; + } catch (err) { + throw err; // see recipe.service.ts's equivalent catch comment + } +} + +/** Throws `404 TECH_STEP_NOT_FOUND` if any id in `ids` doesn't match a reference `TechStep` row — same shape as `recipe.service.ts`'s `assertIngredientsExist`/`assertUnitsExist` for the recipe payload's own reference ids. */ +async function assertTechStepsExist(ids: number[]): Promise { + try { + if (ids.length === 0) return; + const found = await prisma.techStep.findMany({ + where: { id: { in: ids } }, + select: { id: true }, + }); + const foundIds = new Set(found.map((techStep) => techStep.id)); + const missing = ids.filter((id) => !foundIds.has(id)); + if (missing.length > 0) { + throw new HttpError( + 404, + ErrorCode.TECH_STEP_NOT_FOUND, + `TechStep ids not found: ${missing.join(", ")}`, + ); + } + } catch (err) { + throw err; // see loadVisibleStepOrThrow's catch comment + } +} + +function toCorrectionView(correction: CorrectionWithTechSteps): StepTechStepCorrectionView { + return { + id: correction.id, + start: correction.start, + end: correction.end, + previousTechStep: correction.previousTechStep + ? { id: correction.previousTechStep.id, key: correction.previousTechStep.key } + : null, + correctedTechStep: correction.correctedTechStep + ? { id: correction.correctedTechStep.id, key: correction.correctedTechStep.key } + : null, + createdAt: correction.createdAt.toISOString(), + }; +} + +/** + * Records one correction to `stepId`'s detected techniques, submitted by + * `correctorId` — see {@link SubmitTechStepCorrectionInput}'s doc comment + * (`packages/shared`) for what `previousTechStepId`/`correctedTechStepId` + * each mean. Never edited/deleted afterward (see `StepTechStepCorrection`'s + * schema doc comment) — this is a pure insert. + * + * @throws {HttpError} `404 STEP_NOT_FOUND`/`404 RECIPE_NOT_FOUND` — see + * {@link loadVisibleStepOrThrow}. `400 INVALID_CORRECTION_SPAN` if + * `start`/`end` fall outside the step's current `description` (it may + * have been edited since the user last saw it). `404 TECH_STEP_NOT_FOUND` + * if either tech-step id doesn't exist. + */ +export async function submitTechStepCorrection( + recipeId: number, + stepId: number, + input: SubmitTechStepCorrectionInput, + correctorId: number, + viewerHouseId: number | null, +): Promise { + try { + const step = await loadVisibleStepOrThrow(recipeId, stepId, correctorId, viewerHouseId); + + if (input.start >= step.descriptionLength || input.end > step.descriptionLength) { + throw new HttpError( + 400, + ErrorCode.INVALID_CORRECTION_SPAN, + `Span [${input.start}, ${input.end}) falls outside step ${stepId}'s description (length ${step.descriptionLength})`, + ); + } + + const techStepIds = [input.previousTechStepId, input.correctedTechStepId].filter( + (id): id is number => id !== null && id !== undefined, + ); + await assertTechStepsExist(techStepIds); + + const created = await prisma.stepTechStepCorrection.create({ + data: { + stepId: step.id, + correctorId, + start: input.start, + end: input.end, + previousTechStepId: input.previousTechStepId ?? null, + correctedTechStepId: input.correctedTechStepId ?? null, + }, + include: correctionInclude, + }); + + return toCorrectionView(created); + } catch (err) { + throw err; // see loadVisibleStepOrThrow's catch comment + } +} + +/** + * Every correction submitted so far for `stepId`, most recent first — + * mainly useful for a user checking what's already been submitted (by + * anyone) for a span before adding another (see `StepTechStepCorrectionView`'s + * doc comment, `packages/shared`). + * + * @throws {HttpError} `404 STEP_NOT_FOUND`/`404 RECIPE_NOT_FOUND` — see {@link loadVisibleStepOrThrow}. + */ +export async function listTechStepCorrections( + recipeId: number, + stepId: number, + viewerId: number, + viewerHouseId: number | null, +): Promise { + try { + const step = await loadVisibleStepOrThrow(recipeId, stepId, viewerId, viewerHouseId); + const corrections = await prisma.stepTechStepCorrection.findMany({ + where: { stepId: step.id }, + orderBy: { createdAt: "desc" }, + include: correctionInclude, + }); + return corrections.map(toCorrectionView); + } catch (err) { + throw err; // see loadVisibleStepOrThrow's catch comment + } +} diff --git a/apps/api/src/modules/recipe/recipe.routes.ts b/apps/api/src/modules/recipe/recipe.routes.ts index 669bdc7..f7fa8fd 100644 --- a/apps/api/src/modules/recipe/recipe.routes.ts +++ b/apps/api/src/modules/recipe/recipe.routes.ts @@ -4,6 +4,7 @@ import { createRecipeSchema, ErrorCode, listRecipesSchema, + submitTechStepCorrectionSchema, updateRecipeSchema, } from "@batch-cooking/shared"; import { Router } from "express"; @@ -17,6 +18,10 @@ import { removeFavorite, updateRecipe, } from "./recipe.service.js"; +import { + listTechStepCorrections, + submitTechStepCorrection, +} from "./recipe-tech-step-correction.service.js"; /** Router mounted at `/recipes` in app.ts. Every route requires a session — the catalog is shared across households, not public (same reasoning as `planning`/`house`: it's app content, not signup-time reference data). */ export const recipeRouter = Router(); @@ -30,6 +35,15 @@ function parseRecipeId(rawId: string | undefined): number { return id; } +/** Same shape as {@link parseRecipeId}, for the `:stepId` route param of the tech-step-correction routes below — a distinct function only so the error message names the right param. */ +function parseStepId(rawId: string | undefined): number { + const id = Number(rawId); + if (!Number.isInteger(id)) { + throw new HttpError(400, ErrorCode.VALIDATION_ERROR, "stepId must be an integer"); + } + return id; +} + recipeRouter.get( "/", requireAuth, @@ -109,3 +123,29 @@ recipeRouter.delete( res.status(204).end(); }), ); + +// Open to any authenticated viewer who can see the recipe, not just its +// author — see recipe-tech-step-correction.service.ts's own doc comment +// for why. +recipeRouter.post( + "/:id/steps/:stepId/corrections", + requireAuth, + wrapAsyncHandler(async (req, res) => { + const id = parseRecipeId(req.params.id); + const stepId = parseStepId(req.params.stepId); + const input = submitTechStepCorrectionSchema.parse(req.body); + const { id: correctorId, houseId } = res.locals.userProfile; + res.status(201).json(await submitTechStepCorrection(id, stepId, input, correctorId, houseId)); + }), +); + +recipeRouter.get( + "/:id/steps/:stepId/corrections", + requireAuth, + wrapAsyncHandler(async (req, res) => { + const id = parseRecipeId(req.params.id); + const stepId = parseStepId(req.params.stepId); + const { id: viewerId, houseId } = res.locals.userProfile; + res.status(200).json(await listTechStepCorrections(id, stepId, viewerId, houseId)); + }), +); diff --git a/apps/api/src/scripts/backfill-tech-steps.ts b/apps/api/src/scripts/backfill-tech-steps.ts index 519f107..c14e240 100644 --- a/apps/api/src/scripts/backfill-tech-steps.ts +++ b/apps/api/src/scripts/backfill-tech-steps.ts @@ -2,29 +2,29 @@ import { prisma } from "../db/prisma.js"; import { techStepClassifier } from "../lib/recipe-matching/tech-step-matcher.js"; /** - * One-off maintenance script: recomputes every existing `Step`'s - * `StepTechStep` sequence against the *current* classifier - * (`tech-step-matcher.ts`/`tech-step-training-data.ts`), the same way - * `updateRecipe` does when a user resaves a recipe through the UI — - * always `"fr"` (`DEFAULT_TECH_STEP_LOCALE` in `recipe.service.ts`; there's - * no persisted per-recipe locale to recover for a step that already exists, - * so this matches real resave behavior exactly rather than guessing). + * Recomputes every existing `Step`'s `StepTechStep` sequence against the + * *current* classifier (`tech-step-matcher.ts`/`tech-step-training-data.ts`), + * the same way `updateRecipe` does when a user resaves a recipe through the + * UI — always `"fr"` (`DEFAULT_TECH_STEP_LOCALE` in `recipe.service.ts`; + * there's no persisted per-recipe locale to recover for a step that already + * exists, so this matches real resave behavior exactly rather than + * guessing). * * Needed because tech-step detection only ever runs at create/update time * (`recipe.service.ts`'s `matchStepsTechSteps`), never retroactively — a * step saved before a classifier/corpus change (new vocabulary, or the - * `contextStart`/`contextEnd` columns this same session added) keeps - * whatever it was matched with at the time until it's next resaved. Run - * this after a corpus change to bring every existing step in sync without - * asking users to open and resave every recipe by hand: + * `contextStart`/`contextEnd` columns a previous session added) keeps + * whatever it was matched with at the time until it's next resaved. * - * pnpm --filter api exec tsx src/scripts/backfill-tech-steps.ts + * Exported (not just called from this file's own CLI guard below) so + * `retrain-tech-steps.ts` can run it as one step of its own larger + * maintainer workflow, without shelling out to a second process. * * Safe to re-run: each step's technique sequence is fully replaced (delete * + recreate) from the classifier's current output, same as a real edit — * running it twice in a row with no corpus change in between is a no-op. */ -async function backfillTechSteps(): Promise { +export async function backfillTechSteps(): Promise<{ total: number; changed: number }> { const steps = await prisma.step.findMany({ select: { id: true, description: true } }); console.info(`Recomputing tech steps for ${steps.length} step(s)...`); @@ -49,12 +49,22 @@ async function backfillTechSteps(): Promise { } console.info(`Done — ${changed} step(s) recomputed.`); + return { total: steps.length, changed }; } -backfillTechSteps() - .then(() => prisma.$disconnect()) - .catch(async (err) => { - console.error(err); - await prisma.$disconnect(); - process.exit(1); - }); +// Only runs when this file is executed directly (`tsx +// src/scripts/backfill-tech-steps.ts`), not when `retrain-tech-steps.ts` +// imports `backfillTechSteps` above — the standard ESM "is this the entry +// module" check, first needed in this codebase by that new script; every +// prior script here (`seed-runtime.ts`) was always only ever run directly, +// never imported. +const isMainModule = import.meta.url === `file://${process.argv[1]}`; +if (isMainModule) { + backfillTechSteps() + .then(() => prisma.$disconnect()) + .catch(async (err) => { + console.error(err); + await prisma.$disconnect(); + process.exit(1); + }); +} diff --git a/apps/api/src/scripts/list-pending-training-suggestions.ts b/apps/api/src/scripts/list-pending-training-suggestions.ts new file mode 100644 index 0000000..17089a3 --- /dev/null +++ b/apps/api/src/scripts/list-pending-training-suggestions.ts @@ -0,0 +1,71 @@ +import { prisma } from "../db/prisma.js"; + +/** + * Maintainer-facing report of every `TechStepTrainingSuggestion` still + * `status: "pending"` (`TechStepTrainingSuggestion`'s own schema doc + * comment) — generated by `services/tech-step-llm-worker`'s scheduled + * jobs, from either a user correction or the worker's own low-confidence + * audit (`sourceType`). What a maintainer reads *before* hand-editing + * `tech-step-training-data.ts` and running `retrain-tech-steps.ts` — this + * script never writes anything, purely a read-only report to stdout: + * + * pnpm --filter api exec tsx src/scripts/list-pending-training-suggestions.ts + * + * Grouped by technique key so every suggestion for the same entry in + * `TECH_STEP_TRAINING_DATA` is read together, matching how that file + * itself is organized (one block per technique). + */ +async function listPendingTrainingSuggestions(): Promise { + const suggestions = await prisma.techStepTrainingSuggestion.findMany({ + where: { status: "pending" }, + orderBy: [{ techStepId: "asc" }, { createdAt: "asc" }], + include: { techStep: { select: { key: true } } }, + }); + + if (suggestions.length === 0) { + console.info("No pending training suggestions."); + return; + } + + const byTechStepKey = new Map(); + for (const suggestion of suggestions) { + const key = suggestion.techStep.key; + const group = byTechStepKey.get(key); + if (group) { + group.push(suggestion); + } else { + byTechStepKey.set(key, [suggestion]); + } + } + + const lines: string[] = [`# Pending tech-step training suggestions (${suggestions.length})`, ""]; + for (const [techStepKey, group] of byTechStepKey) { + lines.push(`## ${techStepKey}`, ""); + for (const suggestion of group) { + const source = + suggestion.sourceCorrectionId !== null + ? `${suggestion.sourceType} (correction #${suggestion.sourceCorrectionId})` + : suggestion.sourceType; + lines.push(`- id ${suggestion.id} · locale ${suggestion.locale} · source: ${source}`); + if (suggestion.suggestedSynonyms.length > 0) { + lines.push(` - synonyms: ${suggestion.suggestedSynonyms.join(", ")}`); + } + if (suggestion.suggestedUtterances.length > 0) { + lines.push( + ` - utterances: ${suggestion.suggestedUtterances.map((u) => `"${u}"`).join(", ")}`, + ); + } + } + lines.push(""); + } + + console.info(lines.join("\n")); +} + +listPendingTrainingSuggestions() + .then(() => prisma.$disconnect()) + .catch(async (err) => { + console.error(err); + await prisma.$disconnect(); + process.exit(1); + }); diff --git a/apps/api/src/scripts/retrain-tech-steps.ts b/apps/api/src/scripts/retrain-tech-steps.ts new file mode 100644 index 0000000..81de94e --- /dev/null +++ b/apps/api/src/scripts/retrain-tech-steps.ts @@ -0,0 +1,94 @@ +import { prisma } from "../db/prisma.js"; +import { + MIN_OVERALL_F1, + runTechStepEvalSuite, +} from "../lib/recipe-matching/tech-step-eval-runner.js"; +import { backfillTechSteps } from "./backfill-tech-steps.js"; + +/** Parses `--applied=1,2,3`/`--rejected=4,5` from argv into id arrays — both optional, both empty by default (a run with neither flag only re-gates + backfills, doesn't touch any suggestion's status). */ +function parseSuggestionIds(flag: "applied" | "rejected"): number[] { + const prefix = `--${flag}=`; + const arg = process.argv.find((value) => value.startsWith(prefix)); + if (arg === undefined) return []; + return arg + .slice(prefix.length) + .split(",") + .map((value) => value.trim()) + .filter((value) => value.length > 0) + .map((value) => { + const id = Number(value); + if (!Number.isInteger(id)) { + throw new Error(`--${flag}: "${value}" is not a valid integer id`); + } + return id; + }); +} + +/** + * Maintainer workflow closing the loop on a training-corpus change (see + * this feature's plan document): + * + * 1. A maintainer has already hand-edited `tech-step-training-data.ts` + * (informed by `list-pending-training-suggestions.ts`'s report), and + * decided which `TechStepTrainingSuggestion` ids they incorporated + * (`--applied=`) or explicitly discarded (`--rejected=`). + * 2. This script re-runs the F1 regression gate + * ({@link runTechStepEvalSuite} against {@link MIN_OVERALL_F1}) — + * refuses to backfill at all if the edited corpus scores worse than + * the floor, so a bad edit never reaches every existing recipe. + * 3. Backfills every `Step`'s `StepTechStep` sequence against the new + * corpus ({@link backfillTechSteps}). + * 4. Marks the given suggestion ids `applied`/`rejected`, so + * `list-pending-training-suggestions.ts`'s next report doesn't + * surface them again. + * + * Usage: + * + * pnpm --filter api exec tsx src/scripts/retrain-tech-steps.ts --applied=12,13 --rejected=14 + * + * `--applied`/`--rejected` are both optional — omitting both still runs + * the gate + backfill, just leaves every suggestion's `status` untouched + * (useful for re-running the backfill alone after a corpus edit made with + * no suggestions involved at all). + */ +async function retrainTechSteps(): Promise { + const appliedIds = parseSuggestionIds("applied"); + const rejectedIds = parseSuggestionIds("rejected"); + + console.info("Evaluating the current classifier against the labeled evaluation set..."); + const { overall } = await runTechStepEvalSuite(); + console.info( + `F1 ${overall.f1.toFixed(3)} (precision ${overall.precision.toFixed(3)}, recall ${overall.recall.toFixed(3)})`, + ); + if (overall.f1 < MIN_OVERALL_F1) { + throw new Error( + `Aggregate F1 ${overall.f1.toFixed(3)} is below the ${MIN_OVERALL_F1} regression floor — refusing to backfill. Revert or fix the corpus change and re-run.`, + ); + } + + const { total, changed } = await backfillTechSteps(); + console.info(`Backfilled ${changed}/${total} step(s).`); + + if (appliedIds.length > 0) { + await prisma.techStepTrainingSuggestion.updateMany({ + where: { id: { in: appliedIds } }, + data: { status: "applied" }, + }); + console.info(`Marked ${appliedIds.length} suggestion(s) as applied.`); + } + if (rejectedIds.length > 0) { + await prisma.techStepTrainingSuggestion.updateMany({ + where: { id: { in: rejectedIds } }, + data: { status: "rejected" }, + }); + console.info(`Marked ${rejectedIds.length} suggestion(s) as rejected.`); + } +} + +retrainTechSteps() + .then(() => prisma.$disconnect()) + .catch(async (err) => { + console.error(err); + await prisma.$disconnect(); + process.exit(1); + }); diff --git a/apps/api/test/internal/tech-step-worker.routes.test.ts b/apps/api/test/internal/tech-step-worker.routes.test.ts new file mode 100644 index 0000000..b969270 --- /dev/null +++ b/apps/api/test/internal/tech-step-worker.routes.test.ts @@ -0,0 +1,306 @@ +import { ErrorCode } from "@batch-cooking/shared"; +import { expect } from "chai"; +import request from "supertest"; +import { createApp } from "../../src/app.js"; +import { env } from "../../src/config/env.js"; +import { prisma } from "../../src/db/prisma.js"; +import { resetDatabase } from "../../test-support/reset-db.js"; + +const SECRET_HEADER = "X-Internal-Worker-Secret"; + +/** Resolves a reference tech step's id by its `reference-seed-data.ts` uid (also its DB `key`) — mirrors `recipe.test.ts`'s own `techStepId` helper. */ +async function techStepId(key: string): Promise { + const techStep = await prisma.techStep.findFirstOrThrow({ where: { key } }); + return techStep.id; +} + +/** A minimal author + recipe + step fixture — these routes have no notion of a session/viewer, so nothing here needs to go through `/auth/signup` the way `recipe.test.ts`'s fixtures do. */ +async function createRecipeWithStep( + description = "Faire mijoter la sauce.", +): Promise<{ stepId: number; recipeId: number }> { + const author = await prisma.userProfile.create({ + data: { + firstName: "Test", + lastName: "Author", + email: `${crypto.randomUUID()}@example.test`, + passwordHash: "not-a-real-hash", + }, + }); + const recipe = await prisma.recipe.create({ + data: { + name: "Recette", + authorId: author.id, + portions: 4, + steps: { create: [{ description, order: 0 }] }, + }, + include: { steps: true }, + }); + const step = recipe.steps[0]; + if (!step) throw new Error("expected the fixture recipe to have one step"); + return { stepId: step.id, recipeId: recipe.id }; +} + +describe("Internal tech-step worker routes", () => { + const app = createApp(); + + beforeEach(async () => { + await resetDatabase(); + }); + + after(async () => { + await prisma.$disconnect(); + }); + + describe("requireInternalWorker", () => { + it("rejects a request with no secret header with 401 NOT_AUTHENTICATED", async () => { + const res = await request(app).get("/internal/tech-steps/audit-batch"); + + expect(res.status).to.equal(401); + expect(res.body.code).to.equal(ErrorCode.NOT_AUTHENTICATED); + }); + + it("rejects a request with the wrong secret with 401 NOT_AUTHENTICATED", async () => { + const res = await request(app) + .get("/internal/tech-steps/audit-batch") + .set(SECRET_HEADER, "definitely-not-the-right-secret"); + + expect(res.status).to.equal(401); + expect(res.body.code).to.equal(ErrorCode.NOT_AUTHENTICATED); + }); + + it("rejects even the correct secret with 401 NOT_AUTHENTICATED on a plain user-facing route (no bypass of requireAuth)", async () => { + const res = await request(app) + .get("/recipes") + .query({ tab: "publique" }) + .set(SECRET_HEADER, env.INTERNAL_WORKER_SECRET ?? "irrelevant-unset-in-this-env"); + + expect(res.status).to.equal(401); + expect(res.body.code).to.equal(ErrorCode.NOT_AUTHENTICATED); + }); + }); + + // Every test below needs a real configured secret to exercise the success + // path — skipped (not failed) in an environment that hasn't set one, same + // "optional, but the surface fails closed without it" posture + // `INTERNAL_WORKER_SECRET` itself has (see config/env.ts). Both this + // repo's `.env.test.example` and `.github/workflows/ci.yml` set one, so + // this only actually skips in an environment that deliberately diverges + // from both. + describe("with a configured secret", () => { + before(function skipWithoutConfiguredSecret() { + if (env.INTERNAL_WORKER_SECRET === undefined) { + // biome-ignore lint/suspicious/noExplicitAny: mocha's `this.skip()` isn't typed without @types/mocha (not a dependency here) — same untyped-`this` shape a plain JS mocha callback would have. + (this as any).skip(); + } + }); + + function withSecret(req: request.Test): request.Test { + return req.set(SECRET_HEADER, env.INTERNAL_WORKER_SECRET as string); + } + + describe("GET /internal/tech-steps/audit-batch", () => { + // A true low-confidence positive case can't be asserted here without + // a live trained classifier to verify the exact sentence against + // first — same limitation `tech-step-eval-dataset.ts` documents for + // the same reason (no local Postgres was reachable in the session + // that introduced this file). This test instead covers the + // deterministic negative: a step the classifier confidently resolves + // (proven by `tech-step-matcher.test.ts`'s own identical-sentence + // case) must produce zero audit entries — nothing here should ever + // flag a confident match as worth a second opinion. + it("finds nothing to audit in a step the classifier confidently resolves", async () => { + await createRecipeWithStep("Faire mijoter à feu doux"); + + const res = await withSecret( + request(app).get("/internal/tech-steps/audit-batch").query({ locale: "fr" }), + ); + + expect(res.status).to.equal(200); + expect(res.body).to.deep.equal([]); + }); + + it("finds nothing to audit in a step naming no technique at all", async () => { + await createRecipeWithStep("Ranger les couverts dans le tiroir"); + + const res = await withSecret( + request(app).get("/internal/tech-steps/audit-batch").query({ locale: "fr" }), + ); + + expect(res.status).to.equal(200); + expect(res.body).to.deep.equal([]); + }); + + it("rejects a non-positive limit with 400 VALIDATION_ERROR", async () => { + const res = await withSecret( + request(app).get("/internal/tech-steps/audit-batch").query({ limit: 0 }), + ); + + expect(res.status).to.equal(400); + expect(res.body.code).to.equal(ErrorCode.VALIDATION_ERROR); + }); + }); + + describe("GET /internal/tech-steps/pending-corrections", () => { + it("returns unconsumed corrections, oldest first, excluding already-consumed ones", async () => { + const { stepId, recipeId } = await createRecipeWithStep(); + const simmerId = await techStepId("simmer"); + const author = await prisma.recipe + .findUniqueOrThrow({ where: { id: recipeId } }) + .then((recipe) => recipe.authorId); + + const older = await prisma.stepTechStepCorrection.create({ + data: { stepId, correctorId: author, start: 6, end: 13, correctedTechStepId: simmerId }, + }); + const consumed = await prisma.stepTechStepCorrection.create({ + data: { + stepId, + correctorId: author, + start: 0, + end: 5, + correctedTechStepId: simmerId, + consumedAt: new Date(), + }, + }); + const newer = await prisma.stepTechStepCorrection.create({ + data: { stepId, correctorId: author, start: 6, end: 13, correctedTechStepId: simmerId }, + }); + + const res = await withSecret(request(app).get("/internal/tech-steps/pending-corrections")); + + expect(res.status).to.equal(200); + const ids = (res.body as Array<{ id: number }>).map((entry) => entry.id); + expect(ids).to.deep.equal([older.id, newer.id]); + expect(ids).to.not.include(consumed.id); + }); + + it("respects ?limit=", async () => { + const { stepId, recipeId } = await createRecipeWithStep(); + const simmerId = await techStepId("simmer"); + const author = await prisma.recipe + .findUniqueOrThrow({ where: { id: recipeId } }) + .then((recipe) => recipe.authorId); + await prisma.stepTechStepCorrection.createMany({ + data: [ + { stepId, correctorId: author, start: 0, end: 5, correctedTechStepId: simmerId }, + { stepId, correctorId: author, start: 6, end: 13, correctedTechStepId: simmerId }, + ], + }); + + const res = await withSecret( + request(app).get("/internal/tech-steps/pending-corrections").query({ limit: 1 }), + ); + + expect(res.status).to.equal(200); + expect(res.body).to.have.length(1); + }); + }); + + describe("POST /internal/tech-steps/training-suggestions", () => { + it("creates a suggestion and marks its source correction consumed", async () => { + const { stepId, recipeId } = await createRecipeWithStep(); + const simmerId = await techStepId("simmer"); + const author = await prisma.recipe + .findUniqueOrThrow({ where: { id: recipeId } }) + .then((recipe) => recipe.authorId); + const correction = await prisma.stepTechStepCorrection.create({ + data: { stepId, correctorId: author, start: 6, end: 13, correctedTechStepId: simmerId }, + }); + + const res = await withSecret( + request(app) + .post("/internal/tech-steps/training-suggestions") + .send({ + suggestions: [ + { + techStepKey: "simmer", + locale: "fr", + suggestedSynonyms: ["frémissonner"], + suggestedUtterances: ["laisser frémissonner à feu très doux"], + sourceType: "correction", + sourceCorrectionId: correction.id, + }, + ], + }), + ); + + expect(res.status).to.equal(201); + expect(res.body).to.deep.equal({ created: 1 }); + + const suggestions = await prisma.techStepTrainingSuggestion.findMany({ + where: { techStepId: simmerId }, + }); + expect(suggestions).to.have.length(1); + expect(suggestions[0]?.sourceCorrectionId).to.equal(correction.id); + expect(suggestions[0]?.status).to.equal("pending"); + + const updatedCorrection = await prisma.stepTechStepCorrection.findUniqueOrThrow({ + where: { id: correction.id }, + }); + expect(updatedCorrection.consumedAt).to.not.equal(null); + }); + + it("accepts an llm_audit suggestion with no sourceCorrectionId", async () => { + const res = await withSecret( + request(app) + .post("/internal/tech-steps/training-suggestions") + .send({ + suggestions: [ + { + techStepKey: "boil", + locale: "fr", + suggestedSynonyms: ["bouillonner"], + suggestedUtterances: [], + sourceType: "llm_audit", + }, + ], + }), + ); + + expect(res.status).to.equal(201); + expect(res.body).to.deep.equal({ created: 1 }); + }); + + it("rejects sourceType 'correction' with no sourceCorrectionId with 400 VALIDATION_ERROR", async () => { + const res = await withSecret( + request(app) + .post("/internal/tech-steps/training-suggestions") + .send({ + suggestions: [ + { + techStepKey: "boil", + locale: "fr", + suggestedSynonyms: ["bouillonner"], + suggestedUtterances: [], + sourceType: "correction", + }, + ], + }), + ); + + expect(res.status).to.equal(400); + expect(res.body.code).to.equal(ErrorCode.VALIDATION_ERROR); + }); + + it("rejects an unknown techStepKey with 404 TECH_STEP_NOT_FOUND", async () => { + const res = await withSecret( + request(app) + .post("/internal/tech-steps/training-suggestions") + .send({ + suggestions: [ + { + techStepKey: "not-a-real-tech-step", + locale: "fr", + suggestedSynonyms: [], + suggestedUtterances: [], + sourceType: "llm_audit", + }, + ], + }), + ); + + expect(res.status).to.equal(404); + expect(res.body.code).to.equal(ErrorCode.TECH_STEP_NOT_FOUND); + }); + }); + }); +}); diff --git a/apps/api/test/recipe-matching/tech-step-eval.test.ts b/apps/api/test/recipe-matching/tech-step-eval.test.ts new file mode 100644 index 0000000..d6a994f --- /dev/null +++ b/apps/api/test/recipe-matching/tech-step-eval.test.ts @@ -0,0 +1,50 @@ +import { expect } from "chai"; +import { prisma } from "../../src/db/prisma.js"; +import { + MIN_OVERALL_F1, + runTechStepEvalSuite, +} from "../../src/lib/recipe-matching/tech-step-eval-runner.js"; +import { resetDatabase } from "../../test-support/reset-db.js"; + +/** + * Regression gate for `TECH_STEP_TRAINING_DATA` — every change to that + * corpus (including a maintainer applying suggestions from + * `TechStepTrainingSuggestion`, see `scripts/retrain-tech-steps.ts`) must + * keep this suite green. Runs {@link runTechStepEvalSuite} (the real + * trained classifier against `tech-step-eval-dataset.ts`) and asserts the + * aggregate F1 doesn't fall below {@link MIN_OVERALL_F1}. + * + * `MIN_OVERALL_F1` (`tech-step-eval-runner.ts`) is a provisional floor, + * not a target: most of the dataset's cases are built around a + * technique's own registered synonym, which `_classifyClause` always + * resolves correctly via its NER-anchor fallback even when the intent + * classifier itself scores under `CONFIDENCE_THRESHOLD` (see + * `tech-step-matcher.ts`'s doc comment, point 3) — so a healthy run should + * land well above this floor. It's set low enough to tolerate the residual + * uncertainty in a dataset authored without being able to run it against a + * live trained classifier first (no local Postgres was reachable in the + * session that introduced this file — see this feature's plan document). + * Once this suite has actually run once (locally or in CI) and produced + * real numbers, tighten that constant to just below the observed F1, so a + * real future regression still fails loudly instead of hiding under a + * floor that's too forgiving. + */ + +describe("tech-step-eval", () => { + beforeEach(async () => { + await resetDatabase(); + }); + + after(async () => { + await prisma.$disconnect(); + }); + + it(`scores at least ${MIN_OVERALL_F1} aggregate F1 against the labeled evaluation set`, async () => { + const { overall, byKey } = await runTechStepEvalSuite(); + + expect( + overall.f1, + `aggregate F1 ${overall.f1.toFixed(3)} (precision ${overall.precision.toFixed(3)}, recall ${overall.recall.toFixed(3)}) fell below the ${MIN_OVERALL_F1} floor — per-technique breakdown: ${JSON.stringify(byKey)}`, + ).to.be.at.least(MIN_OVERALL_F1); + }); +}); diff --git a/apps/api/test/recipe/recipe-tech-step-correction.test.ts b/apps/api/test/recipe/recipe-tech-step-correction.test.ts new file mode 100644 index 0000000..6722510 --- /dev/null +++ b/apps/api/test/recipe/recipe-tech-step-correction.test.ts @@ -0,0 +1,241 @@ +import type { SignupInput } from "@batch-cooking/shared"; +import { ErrorCode } from "@batch-cooking/shared"; +import { faker } from "@faker-js/faker"; +import { expect } from "chai"; +import request from "supertest"; +import { createApp } from "../../src/app.js"; +import { prisma } from "../../src/db/prisma.js"; +import { resetDatabase } from "../../test-support/reset-db.js"; + +/** See `auth.test.ts` — generated rather than hardcoded, no test fixture looks like a real person's data. */ +function buildSignupPayload(): SignupInput { + const firstName = faker.person.firstName(); + const lastName = faker.person.lastName(); + return { + firstName, + lastName, + email: faker.internet.email({ firstName, lastName }).toLowerCase(), + password: faker.internet.password({ length: 16 }), + }; +} + +/** Resolves a reference tech step's id by its `reference-seed-data.ts` uid (also its DB `key`) — mirrors `recipe.test.ts`'s own `techStepId` helper. */ +async function techStepId(key: string): Promise { + const techStep = await prisma.techStep.findFirstOrThrow({ where: { key } }); + return techStep.id; +} + +describe("Recipe tech-step corrections", () => { + const app = createApp(); + + async function signup(): Promise<{ agent: ReturnType; profileId: number }> { + const agent = request.agent(app); + const res = await agent.post("/auth/signup").send(buildSignupPayload()); + return { agent, profileId: res.body.id }; + } + + /** A `PUBLIC` recipe with one step — every viewer can see this, so most tests below don't need to juggle visibility on top of the correction logic itself. */ + async function createPublicRecipeWithStep( + authorId: number, + description = "Faire mijoter la sauce.", + ): Promise<{ recipeId: number; stepId: number }> { + const recipe = await prisma.recipe.create({ + data: { + name: "Recette", + authorId, + visibility: "PUBLIC", + portions: 4, + steps: { create: [{ description, order: 0 }] }, + }, + include: { steps: true }, + }); + const step = recipe.steps[0]; + if (!step) throw new Error("expected the fixture recipe to have one step"); + return { recipeId: recipe.id, stepId: step.id }; + } + + beforeEach(async () => { + await resetDatabase(); + }); + + after(async () => { + await prisma.$disconnect(); + }); + + describe("POST /recipes/:id/steps/:stepId/corrections", () => { + it("rejects requests without a session cookie with 401 NOT_AUTHENTICATED", async () => { + const { profileId } = await signup(); + const { recipeId, stepId } = await createPublicRecipeWithStep(profileId); + + const res = await request(app) + .post(`/recipes/${recipeId}/steps/${stepId}/corrections`) + .send({ start: 0, end: 5, correctedTechStepId: await techStepId("simmer") }); + + expect(res.status).to.equal(401); + expect(res.body.code).to.equal(ErrorCode.NOT_AUTHENTICATED); + }); + + it("records a correction adding a missing technique (no previousTechStepId)", async () => { + const { agent, profileId } = await signup(); + const { recipeId, stepId } = await createPublicRecipeWithStep(profileId); + const simmerId = await techStepId("simmer"); + + const res = await agent + .post(`/recipes/${recipeId}/steps/${stepId}/corrections`) + .send({ start: 6, end: 13, correctedTechStepId: simmerId }); + + expect(res.status).to.equal(201); + expect(res.body.previousTechStep).to.equal(null); + expect(res.body.correctedTechStep).to.deep.equal({ id: simmerId, key: "simmer" }); + expect(res.body.start).to.equal(6); + expect(res.body.end).to.equal(13); + }); + + it("records a correction relabeling an existing match (both ids set)", async () => { + const { agent, profileId } = await signup(); + const { recipeId, stepId } = await createPublicRecipeWithStep(profileId); + const simmerId = await techStepId("simmer"); + const boilId = await techStepId("boil"); + + const res = await agent + .post(`/recipes/${recipeId}/steps/${stepId}/corrections`) + .send({ start: 6, end: 13, previousTechStepId: simmerId, correctedTechStepId: boilId }); + + expect(res.status).to.equal(201); + expect(res.body.previousTechStep).to.deep.equal({ id: simmerId, key: "simmer" }); + expect(res.body.correctedTechStep).to.deep.equal({ id: boilId, key: "boil" }); + }); + + it("is not restricted to the recipe's author — any viewer who can see it may correct it", async () => { + const { profileId: authorId } = await signup(); + const { agent: otherAgent } = await signup(); + const { recipeId, stepId } = await createPublicRecipeWithStep(authorId); + + const res = await otherAgent + .post(`/recipes/${recipeId}/steps/${stepId}/corrections`) + .send({ start: 6, end: 13, correctedTechStepId: await techStepId("simmer") }); + + expect(res.status).to.equal(201); + }); + + it("rejects both previousTechStepId and correctedTechStepId absent with 400 VALIDATION_ERROR", async () => { + const { agent, profileId } = await signup(); + const { recipeId, stepId } = await createPublicRecipeWithStep(profileId); + + const res = await agent + .post(`/recipes/${recipeId}/steps/${stepId}/corrections`) + .send({ start: 0, end: 5 }); + + expect(res.status).to.equal(400); + expect(res.body.code).to.equal(ErrorCode.VALIDATION_ERROR); + }); + + it("rejects end <= start with 400 VALIDATION_ERROR", async () => { + const { agent, profileId } = await signup(); + const { recipeId, stepId } = await createPublicRecipeWithStep(profileId); + + const res = await agent + .post(`/recipes/${recipeId}/steps/${stepId}/corrections`) + .send({ start: 5, end: 5, correctedTechStepId: await techStepId("simmer") }); + + expect(res.status).to.equal(400); + expect(res.body.code).to.equal(ErrorCode.VALIDATION_ERROR); + }); + + it("rejects a span past the end of the step's description with 400 INVALID_CORRECTION_SPAN", async () => { + const { agent, profileId } = await signup(); + const description = "Court."; + const { recipeId, stepId } = await createPublicRecipeWithStep(profileId, description); + + const res = await agent.post(`/recipes/${recipeId}/steps/${stepId}/corrections`).send({ + start: 0, + end: description.length + 10, + correctedTechStepId: await techStepId("simmer"), + }); + + expect(res.status).to.equal(400); + expect(res.body.code).to.equal(ErrorCode.INVALID_CORRECTION_SPAN); + }); + + it("rejects an unknown correctedTechStepId with 404 TECH_STEP_NOT_FOUND", async () => { + const { agent, profileId } = await signup(); + const { recipeId, stepId } = await createPublicRecipeWithStep(profileId); + + const res = await agent + .post(`/recipes/${recipeId}/steps/${stepId}/corrections`) + .send({ start: 0, end: 5, correctedTechStepId: 999_999 }); + + expect(res.status).to.equal(404); + expect(res.body.code).to.equal(ErrorCode.TECH_STEP_NOT_FOUND); + }); + + it("rejects a step that exists but isn't visible to the viewer with 404 RECIPE_NOT_FOUND", async () => { + const { profileId: authorId } = await signup(); + const { agent: otherAgent } = await signup(); + const recipe = await prisma.recipe.create({ + data: { + name: "Secrète", + authorId, + portions: 4, + steps: { create: [{ description: "Faire mijoter la sauce.", order: 0 }] }, + }, + include: { steps: true }, + }); + const step = recipe.steps[0]; + if (!step) throw new Error("expected the fixture recipe to have one step"); + + const res = await otherAgent + .post(`/recipes/${recipe.id}/steps/${step.id}/corrections`) + .send({ start: 0, end: 5, correctedTechStepId: await techStepId("simmer") }); + + expect(res.status).to.equal(404); + expect(res.body.code).to.equal(ErrorCode.RECIPE_NOT_FOUND); + }); + + it("rejects a stepId that belongs to a different recipe than the URL's :id with 404 STEP_NOT_FOUND", async () => { + const { agent, profileId } = await signup(); + const { recipeId: otherRecipeId } = await createPublicRecipeWithStep(profileId); + const { stepId } = await createPublicRecipeWithStep(profileId); + + const res = await agent + .post(`/recipes/${otherRecipeId}/steps/${stepId}/corrections`) + .send({ start: 0, end: 5, correctedTechStepId: await techStepId("simmer") }); + + expect(res.status).to.equal(404); + expect(res.body.code).to.equal(ErrorCode.STEP_NOT_FOUND); + }); + }); + + describe("GET /recipes/:id/steps/:stepId/corrections", () => { + it("returns every correction submitted for the step, most recent first", async () => { + const { agent, profileId } = await signup(); + const { recipeId, stepId } = await createPublicRecipeWithStep(profileId); + const simmerId = await techStepId("simmer"); + const boilId = await techStepId("boil"); + + await agent + .post(`/recipes/${recipeId}/steps/${stepId}/corrections`) + .send({ start: 6, end: 13, correctedTechStepId: simmerId }); + await agent + .post(`/recipes/${recipeId}/steps/${stepId}/corrections`) + .send({ start: 6, end: 13, previousTechStepId: simmerId, correctedTechStepId: boilId }); + + const res = await agent.get(`/recipes/${recipeId}/steps/${stepId}/corrections`); + + expect(res.status).to.equal(200); + expect(res.body).to.have.length(2); + expect(res.body[0].correctedTechStep).to.deep.equal({ id: boilId, key: "boil" }); + expect(res.body[1].correctedTechStep).to.deep.equal({ id: simmerId, key: "simmer" }); + }); + + it("returns an empty list when nothing has been submitted yet", async () => { + const { agent, profileId } = await signup(); + const { recipeId, stepId } = await createPublicRecipeWithStep(profileId); + + const res = await agent.get(`/recipes/${recipeId}/steps/${stepId}/corrections`); + + expect(res.status).to.equal(200); + expect(res.body).to.deep.equal([]); + }); + }); +}); diff --git a/apps/web/cypress/component/TechStepCorrectionPopover.cy.tsx b/apps/web/cypress/component/TechStepCorrectionPopover.cy.tsx new file mode 100644 index 0000000..effbb22 --- /dev/null +++ b/apps/web/cypress/component/TechStepCorrectionPopover.cy.tsx @@ -0,0 +1,121 @@ +import "../../src/i18n/i18n"; +import { TechStepCorrectionPopover } from "../../src/features/recipes/steps/TechStepCorrectionPopover"; + +// Mounts the popover in isolation (no StepDescription/selection plumbing +// around it) — same "generic component test" posture as CheckboxOption.cy.tsx, +// but this one needs `../../src/i18n/i18n` imported for its side effect +// (initializes the default i18next instance `useTranslation` falls back to +// with no `` in the tree — see that module's own doc +// comment) since, unlike Checkbox/Radio, this component calls +// `useTranslation()`. + +const cook = { id: 1, key: "cook" }; +const simmer = { id: 3, key: "simmer" }; + +function mountPopover( + overrides: Partial<{ + previousTechStepId: number | null; + onClose: () => void; + onSubmitted: (correction: unknown) => void; + }> = {}, +) { + cy.mount( +
+ {/* A genuinely separate sibling to click for the "outside click closes it" test — clicking blindly at a viewport coordinate would risk still landing inside the popover, which fills most of the mounted area on its own. */} +
+ {})} + onSubmitted={overrides.onSubmitted ?? (() => {})} + /> +
, + ); +} + +describe("TechStepCorrectionPopover", () => { + beforeEach(() => { + cy.intercept("GET", "**/reference/tech-steps", { statusCode: 200, body: [cook, simmer] }).as( + "getTechSteps", + ); + }); + + it("shows the selected text and every technique option once loaded", () => { + mountPopover(); + cy.wait("@getTechSteps"); + + cy.contains(".tech-step-correction-popover__selection", "Cuire").should("be.visible"); + cy.get(".tech-step-correction-popover__list button").should("have.length", 2); + }); + + it("offers a 'no technique here' option only when correcting an existing match", () => { + mountPopover({ previousTechStepId: null }); + cy.wait("@getTechSteps"); + cy.get(".tech-step-correction-popover__remove").should("not.exist"); + + mountPopover({ previousTechStepId: cook.id }); + cy.wait("@getTechSteps"); + cy.get(".tech-step-correction-popover__remove").should("exist"); + }); + + it("submits the selected technique and calls onSubmitted", () => { + // Asserting on the resolved `@submitCorrection` interception below, + // rather than inside this handler — a Chai assertion failing *inside* + // a `cy.intercept` callback surfaces as an opaque "onResponse cannot be + // called twice" Cypress internal error instead of a normal assertion + // failure, found while writing this exact test. + cy.intercept("POST", "**/recipes/2/steps/2/corrections", { + statusCode: 201, + body: { + id: 1, + start: 0, + end: 5, + previousTechStep: null, + correctedTechStep: simmer, + createdAt: new Date().toISOString(), + }, + }).as("submitCorrection"); + const onSubmitted = cy.stub().as("onSubmitted"); + mountPopover({ onSubmitted }); + cy.wait("@getTechSteps"); + + cy.contains(".tech-step-correction-popover__list button", "Mijoter").click(); + + cy.wait("@submitCorrection").its("request.body").should("deep.equal", { + start: 0, + end: 5, + previousTechStepId: null, + correctedTechStepId: simmer.id, + }); + cy.get("@onSubmitted").should("have.been.calledOnce"); + }); + + it("shows an error message and stays open when the submission fails", () => { + cy.intercept("POST", "**/recipes/2/steps/2/corrections", { + statusCode: 404, + body: { code: 4051, message: "TechStep not found" }, + }).as("submitCorrection"); + const onClose = cy.stub().as("onClose"); + mountPopover({ onClose }); + cy.wait("@getTechSteps"); + + cy.contains(".tech-step-correction-popover__list button", "Cuire").click(); + + cy.wait("@submitCorrection"); + cy.get(".field-error").should("be.visible"); + cy.get("@onClose").should("not.have.been.called"); + }); + + it("calls onClose on an outside click", () => { + const onClose = cy.stub().as("onClose"); + mountPopover({ onClose }); + cy.wait("@getTechSteps"); + + cy.get('[data-testid="outside-popover"]').click(); + + cy.get("@onClose").should("have.been.calledOnce"); + }); +}); diff --git a/apps/web/cypress/e2e/recipes.feature b/apps/web/cypress/e2e/recipes.feature index bc0b5d1..cb88867 100644 --- a/apps/web/cypress/e2e/recipes.feature +++ b/apps/web/cypress/e2e/recipes.feature @@ -27,6 +27,18 @@ Feature: Managing a recipe from the catalog When I focus the highlighted technique "Cuire" Then the tooltip should show "Cuire" + Scenario: Corrects a detected technique from its highlight + Given the recipe catalog contains "Omelette" + And recipe 2's detail is available + And the tech steps reference list has options + And correcting step 2's "Cuire" match will succeed + When I visit "/recettes/2" + And I click the highlighted technique "Cuire" + Then I should see the technique correction options + When I choose "Mijoter" as the correct technique + Then the correction request should have been made + And I should see "Merci, votre correction a été enregistrée." + Scenario: Deletes a recipe after a two-step confirmation, then clears the selection Given the recipe catalog contains "Omelette" And recipe 2's detail is available diff --git a/apps/web/cypress/e2e/recipes.ts b/apps/web/cypress/e2e/recipes.ts index 14a75f5..9af2311 100644 --- a/apps/web/cypress/e2e/recipes.ts +++ b/apps/web/cypress/e2e/recipes.ts @@ -68,6 +68,41 @@ Given("deleting recipe 2 will succeed", () => { cy.intercept("DELETE", "**/recipes/2", { statusCode: 204 }).as("deleteRecipe"); }); +// Step 2 is `omeletteDetail`'s "Cuire à la poêle." step, whose only +// existing match is `cook` (id 1) — see that fixture above. The response +// mirrors `StepTechStepCorrectionView` (packages/shared), reassigning the +// match to `simmer` (id 3, "Mijoter" — see `the tech steps reference list +// has options`, reference-data.steps.ts). +Given('correcting step 2\'s "Cuire" match will succeed', () => { + cy.intercept("POST", "**/recipes/2/steps/2/corrections", { + statusCode: 201, + body: { + id: 1, + start: 0, + end: 5, + previousTechStep: { id: 1, key: "cook" }, + correctedTechStep: { id: 3, key: "simmer" }, + createdAt: new Date().toISOString(), + }, + }).as("correction"); +}); + +When("I click the highlighted technique {string}", (text: string) => { + cy.contains(".step-tech-step", text).click(); +}); + +Then("I should see the technique correction options", () => { + cy.get(".tech-step-correction-popover").should("be.visible"); +}); + +When("I choose {string} as the correct technique", (label: string) => { + cy.contains(".tech-step-correction-popover__list button", label).click(); +}); + +Then("the correction request should have been made", () => { + cy.wait("@correction"); +}); + Then("the recipe {string} should not be visible in the table", (name: string) => { cy.contains(".recipe-table__name", name).should("not.exist"); }); diff --git a/apps/web/cypress/support/step_definitions/reference-data.steps.ts b/apps/web/cypress/support/step_definitions/reference-data.steps.ts index f105815..a457b4a 100644 --- a/apps/web/cypress/support/step_definitions/reference-data.steps.ts +++ b/apps/web/cypress/support/step_definitions/reference-data.steps.ts @@ -40,6 +40,21 @@ Given("the sources reference list is empty", () => { cy.intercept("GET", "**/reference/sources", { statusCode: 200, body: [] }); }); +// `id`/`key` pairs mirror `recipes.ts`'s `omeletteDetail` fixture (`cook`, +// id 1, is the step's existing match) plus a second option +// (`simmer`/"Mijoter") for `recipes.feature`'s correction scenario to +// re-assign to — TechStepCorrectionPopover's own picker needs at least two +// choices for that scenario to be a meaningful correction, not a no-op. +Given("the tech steps reference list has options", () => { + cy.intercept("GET", "**/reference/tech-steps", { + statusCode: 200, + body: [ + { id: 1, key: "cook" }, + { id: 3, key: "simmer" }, + ], + }); +}); + // The real first entry (`theMealDb`) mirrors what's actually seeded // (`reference-seed-data.ts`'s `registerAllRecipeSources`/ // `syncRecipeSources`); the second is illustrative only — a future diff --git a/apps/web/src/api/client.ts b/apps/web/src/api/client.ts index dcdca30..67b23b0 100644 --- a/apps/web/src/api/client.ts +++ b/apps/web/src/api/client.ts @@ -19,6 +19,9 @@ import { type SafeUserProfile, type SignupInput, type SourceView, + type StepTechStepCorrectionView, + type SubmitTechStepCorrectionInput, + type TechStepView, type ThemePreference, type UnitView, type UpdateRecipeInput, @@ -179,6 +182,11 @@ export class ApiClient { return this._request("/reference/units"); } + /** Reference list of detected cooking techniques — static, non-administrable (`TechStepCorrectionPopover`'s technique picker). Public — no session required. */ + public getTechSteps(): Promise { + return this._request("/reference/tech-steps"); + } + /** Reference list of implemented recipe sources (onboarding wizard's source step, `/parametres/foyer`) — empty until a concrete source is registered. Public — no session required. */ public getSources(): Promise { return this._request("/reference/sources"); @@ -267,6 +275,26 @@ export class ApiClient { return this._request(`/recipes/${id}/favorite`, { method: "DELETE" }); } + /** Submits a correction to one of `stepId`'s detected techniques — see `SubmitTechStepCorrectionInput`'s doc comment (`packages/shared`) for what `previousTechStepId`/`correctedTechStepId` each mean. Open to any viewer who can see the recipe, not just its author. */ + public submitTechStepCorrection( + recipeId: number, + stepId: number, + input: SubmitTechStepCorrectionInput, + ): Promise { + return this._request(`/recipes/${recipeId}/steps/${stepId}/corrections`, { + method: "POST", + body: JSON.stringify(input), + }); + } + + /** Every correction submitted so far for `stepId`, most recent first. */ + public getTechStepCorrections( + recipeId: number, + stepId: number, + ): Promise { + return this._request(`/recipes/${recipeId}/steps/${stepId}/corrections`); + } + /** Fetches the current user's household (with its member list), or `null` if they don't have one yet. */ public getCurrentHouse(): Promise { return this._request("/house/current"); diff --git a/apps/web/src/features/recipes/RecipeDetailPanel.tsx b/apps/web/src/features/recipes/RecipeDetailPanel.tsx index 065fec7..9ef04b9 100644 --- a/apps/web/src/features/recipes/RecipeDetailPanel.tsx +++ b/apps/web/src/features/recipes/RecipeDetailPanel.tsx @@ -209,7 +209,20 @@ export function RecipeDetailPanel({ {recipe.steps.map((step) => (
  • {step.picture && } - + {/* Tied to `showActions` (not unconditionally on): that flag + already distinguishes a full recipe view from a lightweight + preview (`RecipePickerDialog`'s browsing step, `showActions={false}`) + — offering technique corrections in a quick "pick a recipe + for planning" preview would be more distracting than + useful there, even though the API itself allows it for any + viewer who can see the recipe. */} +
    • ))} diff --git a/apps/web/src/features/recipes/recipes.scss b/apps/web/src/features/recipes/recipes.scss index ed923a0..9689e52 100644 --- a/apps/web/src/features/recipes/recipes.scss +++ b/apps/web/src/features/recipes/recipes.scss @@ -633,6 +633,78 @@ // computes and persists `contextStart`/`contextEnd`, this file just no // longer gives that class any styling to render with. +// --- Tech-step correction (StepDescription.tsx editable mode) --------------- + +// Deliberately *not* `position: absolute` (unlike `.calendar-popover`) — see +// `TechStepCorrectionPopover.tsx`'s doc comment for why this renders inline +// in the document flow right below the step's own description instead of +// floating anchored at the selection. +.tech-step-correction-popover { + margin-top: var(--space-xs); + padding: var(--space-md); + background: var(--color-surface-alt); + border: 1px solid var(--color-border); + border-radius: var(--radius-md); + + &__selection { + font-weight: 600; + margin: 0 0 var(--space-sm); + } + + &__list { + display: flex; + flex-wrap: wrap; + gap: var(--space-xs); + list-style: none; + margin: 0 0 var(--space-sm); + padding: 0; + + button { + padding: 0.3rem 0.6rem; + font-size: var(--font-size-sm); + color: var(--color-text); + background: var(--color-surface); + border: 1px solid var(--color-border); + border-radius: var(--radius-pill); + cursor: pointer; + + &:hover:not(:disabled) { + background: color-mix(in srgb, var(--color-primary) 14%, transparent); + border-color: var(--color-primary); + } + + &:disabled { + opacity: 0.5; + cursor: not-allowed; + } + } + + // Nested (rather than a sibling `&__remove` block) so its border-color + // wins over the plain `button` rule above by class-count specificity, + // no `!important` needed. + .tech-step-correction-popover__remove { + color: var(--color-error); + border-color: var(--color-error); + } + } + + &__cancel { + background: none; + border: none; + color: var(--color-text-muted); + text-decoration: underline; + cursor: pointer; + padding: 0; + font-size: var(--font-size-sm); + } +} + +.step-tech-step-correction-confirmation { + margin-top: var(--space-xs); + font-size: var(--font-size-sm); + color: var(--color-success, var(--color-primary)); +} + // --- Favorite star toggle (detail panel header) ----------------------------- .favorite-star-button { position: absolute; diff --git a/apps/web/src/features/recipes/steps/StepDescription.tsx b/apps/web/src/features/recipes/steps/StepDescription.tsx index 7e5641e..fd1ccdc 100644 --- a/apps/web/src/features/recipes/steps/StepDescription.tsx +++ b/apps/web/src/features/recipes/steps/StepDescription.tsx @@ -1,8 +1,13 @@ -import type { StepTechStepView } from "@batch-cooking/shared"; -import { Fragment } from "react"; +import type { StepTechStepCorrectionView, StepTechStepView } from "@batch-cooking/shared"; +import { Fragment, useRef, useState } from "react"; import { useTranslation } from "react-i18next"; import { Tooltip } from "../../../components/ui/Tooltip"; import { splitDescriptionByTechSteps } from "./highlight-tech-steps"; +import { TechStepCorrectionPopover } from "./TechStepCorrectionPopover"; +import { type TextSelectionRange, useTextSelection } from "./use-text-selection"; + +/** How long the post-submit confirmation message stays visible — long enough to read, short enough not to linger once the user has moved on. */ +const CONFIRMATION_DISPLAY_MS = 4000; /** * A recipe step's description, with every detected technique's exact @@ -24,47 +29,136 @@ import { splitDescriptionByTechSteps } from "./highlight-tech-steps"; * `techStep.key` resolves its tooltip label through `catalog.techSteps.` * i18n, the same pattern every other reference catalog (diets, units, …) * uses for its display text. + * + * `editable` (off by default) additionally lets the viewer select text or + * click an existing highlight to open a {@link TechStepCorrectionPopover} — + * see `use-text-selection.ts` for how a browser selection is translated + * into an absolute `[start, end)` span. When `!editable`, every segment + * renders exactly as before (no extra wrapping elements, no `data-offset`, + * no click handlers) — this mode is purely additive, not a rewrite of the + * read-only rendering. */ export function StepDescription({ description, techSteps, + editable = false, + recipeId, + stepId, }: { description: string; techSteps: StepTechStepView[]; + /** Requires `recipeId`/`stepId` when `true` — omit (or leave `false`) for a read-only view with nothing real to correct against yet (e.g. `RecipeDetailPanel`'s `"loaded-draft"` unsaved-preview branch). */ + editable?: boolean; + recipeId?: number; + stepId?: number; }) { const { t } = useTranslation(); const segments = splitDescriptionByTechSteps(description, techSteps); + const containerRef = useRef(null); + const { getSelectionRange } = useTextSelection(containerRef); + + const [activeCorrection, setActiveCorrection] = useState<{ + range: TextSelectionRange; + selectedText: string; + previousTechStepId: number | null; + } | null>(null); + const [showConfirmation, setShowConfirmation] = useState(false); + + function handleMouseUp() { + if (!editable) return; + const range = getSelectionRange(); + if (!range) return; + setActiveCorrection({ + range, + selectedText: description.slice(range.start, range.end), + previousTechStepId: null, + }); + } + + function handleSubmitted(_correction: StepTechStepCorrectionView) { + setShowConfirmation(true); + window.setTimeout(() => setShowConfirmation(false), CONFIRMATION_DISPLAY_MS); + } + + // Tracks each segment's own absolute start offset into `description` as + // the map below walks them in order — segments are contiguous and cover + // the whole description (see `splitDescriptionByTechSteps`'s doc + // comment), so a running total is exact, no re-derivation needed. + let offset = 0; return ( -

    - {segments.map((segment, index) => { - // A segment's own text/techStep don't uniquely identify it (the - // same word can appear twice in one description) — index is the - // only thing that does, but this list is fully regenerated from - // `description`/`techSteps` on every render (never reordered or - // spliced in place), so using it as part of the key is safe here. - const key = `${index}-${segment.text}`; - if (!segment.techStep) return {segment.text}; + <> +

    + {segments.map((segment, index) => { + const start = offset; + offset += segment.text.length; + // A segment's own text/techStep don't uniquely identify it (the + // same word can appear twice in one description) — index is the + // only thing that does, but this list is fully regenerated from + // `description`/`techSteps` on every render (never reordered or + // spliced in place), so using it as part of the key is safe here. + const key = `${index}-${segment.text}`; - if (!segment.isKeyword) { - // Context-only run — rendered as plain text, same as a segment - // with no technique at all (see this component's doc comment for - // why the wider-clause highlight was turned back off). - return {segment.text}; - } - return ( - - {/* A real - - ); - })} -

    + if (!segment.techStep || !segment.isKeyword) { + // Context-only or plain run — rendered as plain text in + // read-only mode, same as before this component supported + // `editable` at all (see this component's doc comment for why + // the wider-clause highlight itself was turned back off). + // Editable mode still wraps it in a `data-offset` span so a + // selection starting/ending in plain text resolves correctly. + if (!editable) return {segment.text}; + return ( + + {segment.text} + + ); + } + + const techStep = segment.techStep; + return ( + + {/* A real + + ); + })} +

    + {showConfirmation && ( +

    + {t("recipes.techStepCorrection.confirmation")} +

    + )} + {editable && activeCorrection && recipeId !== undefined && stepId !== undefined && ( + setActiveCorrection(null)} + onSubmitted={handleSubmitted} + /> + )} + ); } diff --git a/apps/web/src/features/recipes/steps/TechStepCorrectionPopover.tsx b/apps/web/src/features/recipes/steps/TechStepCorrectionPopover.tsx new file mode 100644 index 0000000..9b19e33 --- /dev/null +++ b/apps/web/src/features/recipes/steps/TechStepCorrectionPopover.tsx @@ -0,0 +1,146 @@ +import { + ErrorCode, + type StepTechStepCorrectionView, + type TechStepView, +} from "@batch-cooking/shared"; +import { useEffect, useRef, useState } from "react"; +import { useTranslation } from "react-i18next"; +import { ApiError, apiClient } from "../../../api/client"; +import { errorMessageService } from "../../../services/error-message.service"; +import type { TextSelectionRange } from "./use-text-selection"; + +/** + * Small non-modal popover letting a viewer assign a technique to a selected + * span of a step's description, or clear/relabel an existing match — + * opened by `StepDescription`'s editable mode. Same `mousedown`-outside- + * close pattern as `PlanningPage`'s `CalendarPopover`, not `Dialog.tsx`'s + * native `` — this is a small, contextual pick-one-option surface, + * not a page-blocking modal. + * + * Rendered inline right below the step's own description block (see + * `StepDescription.tsx`), not floating anchored at the selection's exact + * position — simpler and more robust than tracking a caret-anchored + * position across scroll/resize, at the cost of a little visual distance + * from the selected text itself. + * + * Submitting never changes what's currently highlighted — a correction is + * only ever consumed later, offline, by `services/tech-step-llm-worker` + * and a maintainer's review (see `StepTechStepCorrection`'s schema doc + * comment) — so this only ever confirms the submission, it doesn't try to + * (and can't correctly) predict what the classifier will conclude next. + */ +export function TechStepCorrectionPopover({ + recipeId, + stepId, + selectedText, + range, + previousTechStepId, + onClose, + onSubmitted, +}: { + recipeId: number; + stepId: number; + /** The selected span's own text — shown so the user confirms what they're tagging before picking a technique. */ + selectedText: string; + range: TextSelectionRange; + /** Set when correcting an already-detected match (opened from clicking its highlight) rather than a fresh selection — passed through as-is on submit, and offers a "remove" option `null` doesn't. */ + previousTechStepId: number | null; + onClose: () => void; + onSubmitted: (correction: StepTechStepCorrectionView) => void; +}) { + const { t } = useTranslation(); + const popoverRef = useRef(null); + const [techSteps, setTechSteps] = useState(null); + const [isSubmitting, setIsSubmitting] = useState(false); + const [error, setError] = useState(null); + + useEffect(() => { + let cancelled = false; + apiClient + .getTechSteps() + .then((list) => { + if (!cancelled) setTechSteps(list); + }) + .catch(() => { + if (!cancelled) setTechSteps([]); + }); + return () => { + cancelled = true; + }; + }, []); + + useEffect(() => { + function handleClickOutside(e: MouseEvent) { + if (popoverRef.current && !popoverRef.current.contains(e.target as Node)) { + onClose(); + } + } + document.addEventListener("mousedown", handleClickOutside); + return () => document.removeEventListener("mousedown", handleClickOutside); + }, [onClose]); + + async function submit(correctedTechStepId: number | null) { + setIsSubmitting(true); + setError(null); + try { + const correction = await apiClient.submitTechStepCorrection(recipeId, stepId, { + start: range.start, + end: range.end, + previousTechStepId, + correctedTechStepId, + }); + onSubmitted(correction); + onClose(); + } catch (err) { + const code = err instanceof ApiError ? err.code : ErrorCode.INTERNAL_ERROR; + setError(errorMessageService.getLabel(code)); + setIsSubmitting(false); + } + } + + return ( +
    +

    + {t("recipes.techStepCorrection.selectionLabel", { text: selectedText })} +

    + {techSteps === null ? ( +

    {t("recipes.loading")}

    + ) : ( +
      + {previousTechStepId !== null && ( +
    • + +
      • + )} + {techSteps.map((techStep) => ( +
    • + +
      • + ))} +
    + )} + {error &&

    {error}

    } + +
    + ); +} diff --git a/apps/web/src/features/recipes/steps/use-text-selection.ts b/apps/web/src/features/recipes/steps/use-text-selection.ts new file mode 100644 index 0000000..3d6da88 --- /dev/null +++ b/apps/web/src/features/recipes/steps/use-text-selection.ts @@ -0,0 +1,84 @@ +import { type RefObject, useCallback } from "react"; + +/** A `[start, end)` character range into a step's original `description` string — same convention as `StepTechStepView.start`/`end`. */ +export interface TextSelectionRange { + start: number; + end: number; +} + +/** + * Reads the browser's current text selection, translated into a + * {@link TextSelectionRange} into a step's original `description` string — + * the shape `POST /recipes/:id/steps/:stepId/corrections` expects (see + * `SubmitTechStepCorrectionInput`, `packages/shared`). + * + * Works by walking up from each end of the selection's `Range` to the + * nearest ancestor carrying a `data-offset` attribute — set by + * `StepDescription`'s editable mode on every {@link DescriptionSegment}'s + * own wrapping element (``/`