From e6bb9e2d5059a0fcbfd70666c5d9229e4d45efd4 Mon Sep 17 00:00:00 2001
From: Nicolas <nicolaslef53@gmail.com>
Date: Sat, 22 Aug 2026 13:13:54 +0200
Subject: [PATCH] =?UTF-8?q?fix(tech-steps):=20emp=C3=AAche=20le=20contexte?=
 =?UTF-8?q?=20d'un=20match=20d'avaler=20une=20correction=20manuelle=20vois?=
 =?UTF-8?q?ine?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

La correction manuelle ne s'affichait pas quand elle portait sur du texte
qui n'était pas une technique à l'origine — reproduit en live : une
description avec un seul match auto-détecté ("mijoter") voit son
contexte de clause s'étendre sur toute la description dès que
splitIntoClauses (tech-step-matcher.ts) n'a trouvé qu'un seul candidat
NER (le cas courant), même quand ce candidat n'a aucun rapport avec le
reste du texte. splitDescriptionByTechSteps avançait alors son curseur
jusqu'à la fin de ce contexte large, ce qui faisait purement et
simplement disparaître (silencieusement, sans erreur) toute correction
manuelle ajoutée plus loin dans la même description — un mot pourtant
sans aucun rapport avec la technique auto-détectée.

Le contexte d'un match est purement cosmétique (StepDescription.tsx le
rend identique à du texte brut depuis que sa mise en valeur dédiée a
été désactivée) et ne doit donc jamais coûter son propre highlight à
un *autre* match. splitDescriptionByTechSteps distingue maintenant
deux notions : le chevauchement entre les spans *keyword* stricts de
deux entrées (toujours un vrai conflit, l'entrée la plus tardive est
toujours ignorée, comportement inchangé) et le chevauchement du
contexte *cosmétique* d'une entrée sur le keyword d'une autre (jamais
un vrai conflit désormais : le contexte est simplement rogné pour
laisser la place, plutôt que l'entrée voisine entière étant abandonnée).

Vérifié en conditions réelles (Docker) : une correction manuelle sur
"materiel" dans "Faire mijoter la sauce, puis ranger le materiel."
s'affiche maintenant correctement à côté du highlight auto "mijoter",
et survit à un rechargement complet de la page.

Nouveau test de régression dans highlight-tech-steps.cy.tsx
reproduisant exactement ce cas ; les 18 tests du fichier (dont tous
les cas de contexte/malformation déjà couverts) passent toujours.
---
 .../component/highlight-tech-steps.cy.tsx     | 23 ++++++
 .../recipes/steps/highlight-tech-steps.ts     | 70 ++++++++++++++-----
 2 files changed, 77 insertions(+), 16 deletions(-)

diff --git a/apps/web/cypress/component/highlight-tech-steps.cy.tsx b/apps/web/cypress/component/highlight-tech-steps.cy.tsx
index 7eaa4b1..f06a6c4 100644
--- a/apps/web/cypress/component/highlight-tech-steps.cy.tsx
+++ b/apps/web/cypress/component/highlight-tech-steps.cy.tsx
@@ -140,6 +140,29 @@ describe("splitDescriptionByTechSteps", () => {
     ]);
   });
 
+  it("never lets one match's wider context swallow another match's own keyword span", () => {
+    // The motivating real bug (found via live testing, not invented for
+    // this test): "simmer" is the only NER candidate `splitIntoClauses`
+    // found, so its context spans the *entire* description — before this
+    // was fixed, that wide context advanced `cursor` past 39, silently
+    // dropping "setAside"'s own keyword span (a manual correction on
+    // "materiel", a word with no relation to "simmer" at all) instead of
+    // rendering it.
+    const text = "Faire mijoter la sauce, puis ranger le materiel.";
+    const result = splitDescriptionByTechSteps(text, [
+      techStep("simmer", 1, 6, 13, { start: 0, end: 48 }),
+      techStep("setAside", 2, 39, 47, undefined, "manual"),
+    ]);
+    const keywordSegments = result.filter((s) => s.isKeyword);
+    expect(
+      keywordSegments.map((s) => ({ key: s.techStep?.key, text: s.text, source: s.source })),
+    ).to.deep.equal([
+      { key: "simmer", text: "mijoter", source: "auto" },
+      { key: "setAside", text: "materiel", source: "manual" },
+    ]);
+    expect(result.map((s) => s.text).join("")).to.equal(text);
+  });
+
   describe("with a context span wider than the keyword", () => {
     it("splits into context-before / keyword / context-after around a keyword in the middle of its clause", () => {
       // The motivating example: "Dans une poêle chaude, faire chauffer une
diff --git a/apps/web/src/features/recipes/steps/highlight-tech-steps.ts b/apps/web/src/features/recipes/steps/highlight-tech-steps.ts
index 3edbf18..f98bf52 100644
--- a/apps/web/src/features/recipes/steps/highlight-tech-steps.ts
+++ b/apps/web/src/features/recipes/steps/highlight-tech-steps.ts
@@ -28,34 +28,54 @@ export interface DescriptionSegment {
  * (the keyword) and, when present, `contextStart`/`contextEnd` (the wider
  * clause it was found in — see `StepTechStepView`, resolved server-side by
  * `tech-step-matcher.ts`'s `matchTechStepSpans`). An entry with no context
- * (older data, saved before that column pair existed — see
- * `StepTechStep`'s schema doc comment) degrades to a keyword-only segment,
- * same as before context spans existed at all.
+ * (older data, saved before that column pair existed, or a manual
+ * correction — see `StepTechStep`'s schema doc comment) degrades to a
+ * keyword-only segment, same as before context spans existed at all.
  *
  * `techSteps` is expected already sorted by `start` (the API returns it in
  * `StepTechStep.order`, which *is* reading order — see that model's schema
- * doc comment) but this re-sorts defensively (by context start when
- * present, since context always starts at or before its own keyword)
- * rather than assuming it, and silently drops any entry whose bounds don't
- * make sense against `description` or a previously-accepted entry's own
- * bounds — a malformed/out-of-date span degrades to "just don't highlight
- * that one" rather than a garbled slice or a crash.
+ * doc comment) but this re-sorts defensively by each entry's own tight
+ * `start` rather than assuming it, and silently drops any entry whose own
+ * bounds don't make sense against `description` or a previously-accepted
+ * entry's own tight keyword span — a malformed/out-of-date span degrades to
+ * "just don't highlight that one" rather than a garbled slice or a crash.
+ *
+ * Two entries' tight keyword spans are never allowed to overlap (the later
+ * one is dropped, same as always), but two entries' wider *context* clauses
+ * are allowed to overlap each other and are silently clipped to make room —
+ * context is cosmetic only (`StepDescription.tsx` renders it identically to
+ * plain text) and must never cost a *different* entry its own real keyword
+ * highlight. This matters far more than it looks: a clause `splitIntoClauses`
+ * (`tech-step-matcher.ts`) found only one NER candidate in gets that
+ * candidate's context spanning the *entire* clause — often the entire
+ * description — so without clipping, a single auto-detected match anywhere
+ * in a step could silently swallow every manual correction added anywhere
+ * else in that same step's description, with no error, just an unstyled
+ * word in the rendered text. Found via live testing: a "simmer" match's
+ * whole-description context ate a manual "setAside" correction added to a
+ * later, otherwise-plain word in the same step.
  */
 export function splitDescriptionByTechSteps(
   description: string,
   techSteps: StepTechStepView[],
 ): DescriptionSegment[] {
-  const sorted = [...techSteps].sort(
-    (a, b) => (a.contextStart ?? a.start) - (b.contextStart ?? b.start),
-  );
+  const sorted = [...techSteps].sort((a, b) => a.start - b.start);
 
-  const segments: DescriptionSegment[] = [];
-  let cursor = 0;
-  for (const { techStep, start, end, contextStart, contextEnd, source } of sorted) {
+  // First pass: decide which entries survive at all, using only each
+  // entry's own tight keyword span for the cross-entry overlap check
+  // (`start < keywordCursor`) — the one thing two independent matches are
+  // never allowed to genuinely share. An entry's own context is still
+  // validated against its *own* keyword span here (`wideStart > start`,
+  // `end > wideEnd`, `wideEnd > description.length`) — a self-inconsistent
+  // span is dropped regardless of any other entry.
+  const valid: StepTechStepView[] = [];
+  let keywordCursor = 0;
+  for (const entry of sorted) {
+    const { start, end, contextStart, contextEnd } = entry;
     const wideStart = contextStart ?? start;
     const wideEnd = contextEnd ?? end;
     if (
-      wideStart < cursor ||
+      start < keywordCursor ||
       wideStart > start ||
       start >= end ||
       end > wideEnd ||
@@ -63,6 +83,24 @@ export function splitDescriptionByTechSteps(
     ) {
       continue;
     }
+    valid.push(entry);
+    keywordCursor = end;
+  }
+
+  const segments: DescriptionSegment[] = [];
+  let cursor = 0;
+  for (const [index, entry] of valid.entries()) {
+    const { techStep, start, end, contextStart, contextEnd, source } = entry;
+    const next = valid[index + 1];
+    // Clipped against `cursor` (this entry can't render context over
+    // territory already emitted) and the next surviving entry's own tight
+    // `start` (this entry's context can't reach into a neighbor's real
+    // keyword span) — provably within `[cursor, start]`/`[end, next.start]`
+    // respectively given `valid`'s own non-overlapping-tight-span
+    // invariant from the first pass, so never produces a negative-length
+    // slice.
+    const wideStart = Math.max(contextStart ?? start, cursor);
+    const wideEnd = Math.min(contextEnd ?? end, next?.start ?? description.length);
 
     if (wideStart > cursor) {
       segments.push({