diff --git a/apps/api/.mocharc.json b/apps/api/.mocharc.json index 193fdb9..2a00c64 100644 --- a/apps/api/.mocharc.json +++ b/apps/api/.mocharc.json @@ -2,5 +2,6 @@ "extension": ["ts"], "spec": "test/**/*.test.ts", "node-option": ["import=tsx"], - "timeout": 10000 + "timeout": 10000, + "require": ["test-support/mocha-root-hooks.ts"] } diff --git a/apps/api/test-support/mocha-root-hooks.ts b/apps/api/test-support/mocha-root-hooks.ts new file mode 100644 index 0000000..016869d --- /dev/null +++ b/apps/api/test-support/mocha-root-hooks.ts @@ -0,0 +1,40 @@ +import { techStepClassifier } from "../src/lib/recipe-matching/tech-step-matcher.js"; +import { resetDatabase } from "./reset-db.js"; + +/** + * Mocha root hook plugin (see `.mocharc.json`'s `require`) — runs once + * before every test file's own suites, regardless of load order. + * + * Warms up `techStepClassifier` here, with its own generous timeout, + * instead of leaving it to happen lazily on whichever test file Mocha + * happens to load first. In production this one-time cost (a `POST + * /v1/train` round-trip per locale to `services/tech-step-intent-service`, + * training a real `textcat` on the full `TECH_STEP_TRAINING_DATA` corpus) + * is paid by `server.ts`'s own `techStepClassifier.warmUp()` before the + * server ever accepts traffic — but this test suite builds its `app` + * directly via `createApp()` (see e.g. `tech-step-worker.routes.test.ts`), + * never running `server.ts` at all. Without this hook, that cost instead + * landed inside whichever test's own call happened to trigger + * `_ensureTrained()` first — found the hard way in CI, where training the + * full corpus took longer than a single test's default 10s timeout + * (`.mocharc.json`) and failed an otherwise-unrelated test purely because + * Mocha loaded its file first alphabetically. + * + * `resetDatabase()` runs first, deliberately: `_train()` + * (`tech-step-matcher.ts`) resolves `TechStep.key -> id` from the database + * alongside training, and a freshly-migrated (never-seeded) test database + * has no `TechStep` rows yet — every per-test `beforeEach` in this suite + * already calls `resetDatabase()` again before its own test, which is a + * no-op duplication of effort but not a correctness problem: `TRUNCATE ... + * RESTART IDENTITY` plus deterministic re-seeding (`seedReferenceData`) + * assigns the exact same ids every time, so the `uid -> id` map memoized + * here from this first reset stays valid for every reset after it. + */ +export const mochaHooks = { + // biome-ignore lint/suspicious/noExplicitAny: Mocha's root hook `this` (a Context with `.timeout()`) isn't typed without @types/mocha (not a dependency here) — same untyped-`this` shape already used in tech-step-worker.routes.test.ts. + async beforeAll(this: any): Promise { + this.timeout(60000); + await resetDatabase(); + await techStepClassifier.warmUp(); + }, +}; diff --git a/services/tech-step-intent-service/intent_service/locale_pipeline.py b/services/tech-step-intent-service/intent_service/locale_pipeline.py index 0643df7..5a0c790 100644 --- a/services/tech-step-intent-service/intent_service/locale_pipeline.py +++ b/services/tech-step-intent-service/intent_service/locale_pipeline.py @@ -25,9 +25,9 @@ from dataclasses import dataclass, field import spacy from spacy.language import Language from spacy.matcher import PhraseMatcher -from spacy.tokens import Doc +from spacy.tokens import Doc, Span from spacy.training import Example -from spacy.util import minibatch +from spacy.util import filter_spans, minibatch from .text_normalization import normalize_text @@ -272,14 +272,31 @@ class LocalePipeline: doc = self._base_nlp(text) - entities = [ - Entity( - uid=self._base_nlp.vocab.strings[match_id], - start=doc[start].idx, - end=doc[end - 1].idx + len(doc[end - 1].text), - ) - for match_id, start, end in self._matcher(doc) + # A technique's own synonym list can legitimately contain one phrase + # nested inside another (`melt`'s "fondre" is a literal substring of + # its own "faire fondre") — the `PhraseMatcher` reports *both* as + # separate matches at overlapping positions, which without + # resolution would hand `splitIntoClauses` (apps/api) two candidates + # for what a human reads as one mention, producing the same + # techStepId twice in the final result. `filter_spans` keeps only + # the longest match at each position (so "faire fondre" wins over + # the "fondre" it contains) — found by a real regression in + # `tech-step-matcher.test.ts`'s "detects several distinct + # techniques..." case once this service replaced node-nlp (which + # apparently resolved this internally; nothing here recreates that + # by choice, `filter_spans` is spaCy's own documented tool for + # exactly this "one span per position" problem, e.g. as used for + # NER-style outputs). + matched_spans = [ + Span(doc, start, end, label=match_id) for match_id, start, end in self._matcher(doc) ] + entities = sorted( + ( + Entity(uid=self._base_nlp.vocab.strings[span.label], start=span.start_char, end=span.end_char) + for span in filter_spans(matched_spans) + ), + key=lambda entity: entity.start, + ) cats = doc.cats if not cats: diff --git a/services/tech-step-intent-service/tests/test_locale_pipeline_entities.py b/services/tech-step-intent-service/tests/test_locale_pipeline_entities.py index f108abd..eb2169f 100644 --- a/services/tech-step-intent-service/tests/test_locale_pipeline_entities.py +++ b/services/tech-step-intent-service/tests/test_locale_pipeline_entities.py @@ -24,8 +24,15 @@ _FR_ENTRIES = [ utterances=["préchauffer le four à 180 degrés", "mettre la poêle sur feu vif"], ), TrainEntry( + # `synonyms` deliberately includes both "fondre" (standalone) and + # "faire fondre" (containing it) — mirrors the real corpus + # (`tech-step-training-data.ts`) exactly, and is what + # `test_does_not_double_match_a_synonym_nested_in_a_longer_one` + # below exists to guard: the `PhraseMatcher` reports both as + # separate overlapping matches, `LocalePipeline.process` must + # collapse them into one. uid="melt", - synonyms=["faire fondre", "faire chauffer"], + synonyms=["fondre", "fondu", "faire fondre", "faire chauffer"], utterances=["faire fondre le beurre", "faire chauffer une noix de beurre"], ), TrainEntry( @@ -89,6 +96,21 @@ def test_detects_two_techniques_with_exact_tight_spans_reading_order(fr_pipeline assert text[melt_entity.start : melt_entity.end].lower() == "faire fondre" +def test_does_not_double_match_a_synonym_nested_in_a_longer_one(fr_pipeline: LocalePipeline): + # Regression: "fondre" is itself a substring of "faire fondre" — both + # are registered as `melt` synonyms (like the real corpus). Without + # `filter_spans` in `LocalePipeline.process`, the `PhraseMatcher` + # reports *both* overlapping matches, producing `melt` twice in + # apps/api's final `matchTechSteps` output instead of once (caught by a + # real CI failure in `tech-step-matcher.test.ts` once this service + # replaced node-nlp). + text = "faire fondre le beurre" + result = fr_pipeline.process(text) + assert [entity.uid for entity in result.entities] == ["melt"] + entity = result.entities[0] + assert text[entity.start : entity.end] == "faire fondre" + + def test_matches_the_classic_poele_chaude_example_with_exact_offsets(fr_pipeline: LocalePipeline): # Le cas motivant les context spans côté apps/api (tech-step-matcher.test.ts) : # le mot-clé de `preheat` est un groupe nominal ("poêle chaude"), pas un