fix(recipes): corrige les matches dupliques et le timeout de warm-up des tests CI
Deux bugs reels trouves par la premiere execution CI de la migration node-nlp -> tech-step-intent-service : 1. PhraseMatcher retourne tous les matches y compris chevauchants — un synonyme comme "fondre" litteralement contenu dans "faire fondre" (tous deux synonymes de `melt`) produisait deux candidats separes pour la meme technique, dupliquant son techStepId dans le resultat final. Fixe avec spacy.util.filter_spans (garde le plus long match par position) dans LocalePipeline.process. Test de non-regression ajoute. 2. La suite Mocha construit `app` directement via createApp(), sans jamais passer par server.ts — le warm-up (POST /v1/train fr+en sur le corpus complet) se declenchait donc paresseusement dans le premier test qui appelait le classifieur, depassant le timeout Mocha de 10s par test. Fixe par un root hook plugin Mocha (test-support/mocha-root-hooks.ts, .mocharc.json) qui reset la DB et warm up le classifieur une seule fois avant toute suite, avec son propre timeout de 60s. Verifie : 27/27 tests pytest du service (dont le nouveau test de non-regression), lint + build complets du monorepo. La suite Mocha elle-meme n'a toujours pas pu etre executee dans cet environnement (pas de Postgres disponible ici) — a confirmer via la CI. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
parent
18abae7b6a
commit
690125bec3
4 changed files with 91 additions and 11 deletions
|
|
@ -2,5 +2,6 @@
|
||||||
"extension": ["ts"],
|
"extension": ["ts"],
|
||||||
"spec": "test/**/*.test.ts",
|
"spec": "test/**/*.test.ts",
|
||||||
"node-option": ["import=tsx"],
|
"node-option": ["import=tsx"],
|
||||||
"timeout": 10000
|
"timeout": 10000,
|
||||||
|
"require": ["test-support/mocha-root-hooks.ts"]
|
||||||
}
|
}
|
||||||
|
|
|
||||||
40
apps/api/test-support/mocha-root-hooks.ts
Normal file
40
apps/api/test-support/mocha-root-hooks.ts
Normal file
|
|
@ -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<void> {
|
||||||
|
this.timeout(60000);
|
||||||
|
await resetDatabase();
|
||||||
|
await techStepClassifier.warmUp();
|
||||||
|
},
|
||||||
|
};
|
||||||
|
|
@ -25,9 +25,9 @@ from dataclasses import dataclass, field
|
||||||
import spacy
|
import spacy
|
||||||
from spacy.language import Language
|
from spacy.language import Language
|
||||||
from spacy.matcher import PhraseMatcher
|
from spacy.matcher import PhraseMatcher
|
||||||
from spacy.tokens import Doc
|
from spacy.tokens import Doc, Span
|
||||||
from spacy.training import Example
|
from spacy.training import Example
|
||||||
from spacy.util import minibatch
|
from spacy.util import filter_spans, minibatch
|
||||||
|
|
||||||
from .text_normalization import normalize_text
|
from .text_normalization import normalize_text
|
||||||
|
|
||||||
|
|
@ -272,14 +272,31 @@ class LocalePipeline:
|
||||||
|
|
||||||
doc = self._base_nlp(text)
|
doc = self._base_nlp(text)
|
||||||
|
|
||||||
entities = [
|
# A technique's own synonym list can legitimately contain one phrase
|
||||||
Entity(
|
# nested inside another (`melt`'s "fondre" is a literal substring of
|
||||||
uid=self._base_nlp.vocab.strings[match_id],
|
# its own "faire fondre") — the `PhraseMatcher` reports *both* as
|
||||||
start=doc[start].idx,
|
# separate matches at overlapping positions, which without
|
||||||
end=doc[end - 1].idx + len(doc[end - 1].text),
|
# resolution would hand `splitIntoClauses` (apps/api) two candidates
|
||||||
)
|
# for what a human reads as one mention, producing the same
|
||||||
for match_id, start, end in self._matcher(doc)
|
# 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
|
cats = doc.cats
|
||||||
if not cats:
|
if not cats:
|
||||||
|
|
|
||||||
|
|
@ -24,8 +24,15 @@ _FR_ENTRIES = [
|
||||||
utterances=["préchauffer le four à 180 degrés", "mettre la poêle sur feu vif"],
|
utterances=["préchauffer le four à 180 degrés", "mettre la poêle sur feu vif"],
|
||||||
),
|
),
|
||||||
TrainEntry(
|
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",
|
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"],
|
utterances=["faire fondre le beurre", "faire chauffer une noix de beurre"],
|
||||||
),
|
),
|
||||||
TrainEntry(
|
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"
|
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):
|
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 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
|
# le mot-clé de `preheat` est un groupe nominal ("poêle chaude"), pas un
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue