From f1a4594a0add20e4e21123237e35eb28b61647ea Mon Sep 17 00:00:00 2001 From: Amin Mousavi Date: Mon, 7 Sep 2026 12:42:06 +0800 Subject: [PATCH] fix(voice): validate a code's region against its target properly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Second correctness pass on e271858 found three ways an arch-only appliance could be written as a per-tooth job, plus a gap in the previous repair. 1. The deferred region check was never completed. A mixed-region category resolved with no region recorded, and nothing re-validated the leaf the clinician then picked — "پروتز متحرک برای دندون ۱۲" wrote a complete denture onto tooth 12, which the manual chart cannot produce and which task generation would expand as denture steps. Targets now resolve BEFORE types, so the target kind narrows a category's candidates and an impossible leaf is never offered. The deferral disappears with it. 2. `some` over the stack's regions let one legal code admit every other. `types: ['pfm_crown','night_guard_soft']` on tooth 12 passed, because `crown` suited the tooth, and wrote a night guard onto that tooth. Now every named code must suit the target. 3. `regions.size === 1` also deferred `implant`, whose leaves span root and crown — both tooth regions, so not a tooth/arch category at all. An implant aimed at a jaw resolved as a UA target. Narrowing by target kind rejects it instead. 4. The e271858 type-row lock ticked the row and the payload but not the count, so the sheet read "Apply 1 field" while two landed. One `effectiveSelection` now drives the count and the payload. Also narrows a `filter(Boolean)` in the disjointness test that left two implicit-any errors under the full tsconfig (nest build excludes specs, so the repo gate never saw them). Pre-existing at 15ddb9a. Adds four resolver tests: candidate narrowing per target kind, a category with no usable leaf, a stack where only one code suits, and a legal stack. All three defects passed the previous 209 tests. Gates: backend 212 tests, nest build, ESLint clean on the voice module; frontend 37 Vitest tests, tsc --noEmit, ESLint clean. Co-Authored-By: Claude Opus 5 (1M context) --- .../modules/voice/extraction.resolver.spec.ts | 73 ++++++++- .../src/modules/voice/extraction.resolver.ts | 146 +++++++++++------- .../ui/treatment/VoiceReviewSheet.tsx | 17 +- 3 files changed, 170 insertions(+), 66 deletions(-) diff --git a/backend/src/modules/voice/extraction.resolver.spec.ts b/backend/src/modules/voice/extraction.resolver.spec.ts index a39690f..55dc36d 100644 --- a/backend/src/modules/voice/extraction.resolver.spec.ts +++ b/backend/src/modules/voice/extraction.resolver.spec.ts @@ -281,7 +281,9 @@ describe('resolveProsthesisAssignment', () => { const leafCodes = new Set(PROSTHESIS_TYPES.map((t) => t.code)); const categoryCodes = new Set(PROSTHESIS_TYPES.map((t) => t.category)); const subcategoryCodes = new Set( - PROSTHESIS_TYPES.map((t) => t.subcategory).filter(Boolean), + PROSTHESIS_TYPES.map((t) => t.subcategory).filter( + (code): code is string => Boolean(code), + ), ); for (const code of categoryCodes) expect(leafCodes.has(code)).toBe(false); for (const code of subcategoryCodes) { @@ -334,10 +336,11 @@ describe('resolveProsthesisAssignment', () => { ]); }); - it('defers the region check for a mixed-region category (removable)', () => { - // complete_denture is 'arch', partial_denture is 'crown' — no region to check until a - // leaf is picked. Neither a tooth nor a jaw target may be rejected while it is still - // the bare category. + it('narrows a mixed-region category to the leaves the target can carry', () => { + // complete_denture is 'arch', partial_denture is 'crown'. Deferring the check until a + // leaf was picked left nothing to complete it, and wrote a complete denture onto one + // tooth. The target kind narrows the candidates instead, so an impossible leaf is never + // offered. const toothTarget = resolve({ targets: [tooth('12')], types: ['removable'], @@ -345,7 +348,10 @@ describe('resolveProsthesisAssignment', () => { }); expect(toothTarget.resolved.targets).toEqual(['12']); expect(toothTarget.unresolved).toContainEqual( - expect.objectContaining({ reason: 'prosthesis_type_ambiguous' }), + expect.objectContaining({ + reason: 'prosthesis_type_ambiguous', + candidates: ['partial_denture'], + }), ); expect(toothTarget.unresolved).not.toContainEqual( expect.objectContaining({ reason: 'code_not_valid_for_target' }), @@ -357,11 +363,66 @@ describe('resolveProsthesisAssignment', () => { spoken: 'دنچر فک بالا', }); expect(jawTarget.resolved.targets).toEqual([ARCH_TOOTH_UPPER]); + expect(jawTarget.unresolved).toContainEqual( + expect.objectContaining({ + reason: 'prosthesis_type_ambiguous', + candidates: ['complete_denture'], + }), + ); expect(jawTarget.unresolved).not.toContainEqual( expect.objectContaining({ reason: 'code_not_valid_for_target' }), ); }); + it('rejects a category with no leaf the target can carry', () => { + // `implant` spans root + crown, both tooth regions, so it is not a mixed tooth/arch + // category and must not defer. Aimed at a jaw it is a contradiction. + const { resolved, unresolved } = resolve({ + targets: [positional({ arch: 'upper', spoken: 'فک بالا' })], + types: ['implant'], + spoken: 'ایمپلنت بالا', + }); + expect(resolved.targets).toEqual([]); + expect(resolved.types).toEqual([]); + expect(unresolved).toContainEqual( + expect.objectContaining({ reason: 'code_not_valid_for_target' }), + ); + expect(unresolved).not.toContainEqual( + expect.objectContaining({ reason: 'prosthesis_type_ambiguous' }), + ); + }); + + it('rejects a stack where only one code suits the target', () => { + // One legal crown must not admit an arch-only appliance onto the same tooth. Validating + // with `some` over the stack's regions did exactly that. + const { resolved, unresolved } = resolve({ + targets: [tooth('12')], + types: ['pfm_crown', 'night_guard_soft'], + spoken: 'دندون ۱۲ روکش و نایت گارد', + }); + expect(resolved.targets).toEqual([]); + expect(unresolved).toContainEqual( + expect.objectContaining({ + spoken: '12', + reason: 'code_not_valid_for_target', + }), + ); + }); + + it('accepts a stack whose every code suits the target', () => { + const { resolved, unresolved } = resolve({ + targets: [tooth('12')], + types: ['zirconia_abutment', 'monolithic_zirconia'], + spoken: 'دندون ۱۲ ایمپلنت با روکش زیرکونیا', + }); + expect(resolved.targets).toEqual(['12']); + expect(resolved.types).toEqual([ + 'zirconia_abutment', + 'monolithic_zirconia', + ]); + expect(unresolved).toEqual([]); + }); + it('a target with no types resolves with an empty types[], and does not fail the assignment', () => { const { resolved } = resolve({ targets: [tooth('13'), tooth('14')], diff --git a/backend/src/modules/voice/extraction.resolver.ts b/backend/src/modules/voice/extraction.resolver.ts index 4990415..dce2bd6 100644 --- a/backend/src/modules/voice/extraction.resolver.ts +++ b/backend/src/modules/voice/extraction.resolver.ts @@ -349,45 +349,19 @@ export function resolveProsthesisAssignment( ctx: ResolveContext, unresolved: UnresolvedItem[], ): ResolvedProsthesisAssignment { - const rawTypes = Array.isArray(assignment?.types) ? assignment.types : []; - const leafCodes = new Set(); - const regionSet = new Set(); - - for (const rawCode of rawTypes) { - const classification = classifyTypeCode(rawCode, ctx); - if (classification.kind === 'leaf') { - leafCodes.add(classification.code); - regionSet.add(classification.chartRegion); - } else if ( - classification.kind === 'category' || - classification.kind === 'subcategory' - ) { - unresolved.push({ - spoken: spokenOf(assignment) || classification.code, - reason: 'prosthesis_type_ambiguous', - candidates: [...classification.leafCodes].sort(), - assignmentIndex: index, - }); - // A category whose leaves share one region is informative even before the leaf is - // picked; a mixed one (only `removable` today) defers the region check entirely - // (decision 49) rather than risk `code_not_valid_for_target` on a category that has - // not been narrowed yet. - if (classification.regions.size === 1) { - for (const region of classification.regions) regionSet.add(region); - } - } else if (typeof rawCode === 'string' && rawCode.trim()) { - unresolved.push({ - spoken: rawCode, - reason: 'unknown_catalog_code', - assignmentIndex: index, - }); - } - } - + // Targets are resolved BEFORE types, because the target kind is what narrows a spoken + // category. "پروتز متحرک برای دندون ۱۲" must offer only `partial_denture` — the one + // `removable` leaf with a tooth region — not the whole category. Deferring the region check + // until a leaf is picked (the earlier reading of decision 49) left nothing to complete it, + // and wrote a complete denture onto a single tooth. const rawTargets = Array.isArray(assignment?.targets) ? assignment.targets : []; - const targets = new Set(); + const resolvedTargets: { + kind: 'tooth' | 'jaw'; + spoken: string; + codes: string[]; + }[] = []; for (const rawTarget of rawTargets) { const resolution = resolveAssignmentTarget(rawTarget); @@ -407,31 +381,99 @@ export function resolveProsthesisAssignment( continue; } - // Nothing to validate against yet — a bare category is deferred, and an empty types[] - // means the target is simply named with no job (left to the caller to render struck - // through, using `teeth` minus every assignment's resolved targets). - const valid = - regionSet.size === 0 || - [...regionSet].some((region) => - acceptableRegions(resolution.kind).has(region), - ); - if (!valid) { + resolvedTargets.push( + resolution.kind === 'tooth' + ? { kind: 'tooth', spoken, codes: [resolution.fdi] } + : { kind: 'jaw', spoken, codes: archSentinels(resolution.arch) }, + ); + } + + // The regions any resolved target of this assignment can carry. Empty means no target + // resolved yet — a bare "نایت گارد" with no jaw — and then nothing can be narrowed or + // validated, so every leaf stays on offer. + const allowedRegions = new Set(); + for (const target of resolvedTargets) { + for (const region of acceptableRegions(target.kind)) + allowedRegions.add(region); + } + const suits = (region: string) => + allowedRegions.size === 0 || allowedRegions.has(region); + + const rawTypes = Array.isArray(assignment?.types) ? assignment.types : []; + const leafCodes = new Set(); + const leafRegions = new Set(); + // Set when a named type cannot suit this assignment's targets at all. The targets are then + // dropped without a second report — one `code_not_valid_for_target` per contradiction, not + // one per target — so the assignment applies nothing rather than half of what was said. + let regionConflict = false; + + for (const rawCode of rawTypes) { + const classification = classifyTypeCode(rawCode, ctx); + + if (classification.kind === 'leaf') { + leafCodes.add(classification.code); + leafRegions.add(classification.chartRegion); + continue; + } + + if ( + classification.kind === 'category' || + classification.kind === 'subcategory' + ) { + const leaves = classification.leafCodes.filter((code) => { + const leaf = ctx.prosthesisLeaves.find((l) => l.code === code); + return leaf ? suits(leaf.chartRegion) : false; + }); + + // A category with no leaf this target can carry is a contradiction, not an ambiguity: + // `implant` aimed at a jaw, or `appliance` aimed at a tooth. + if (leaves.length === 0) { + unresolved.push({ + spoken: spokenOf(assignment) || classification.code, + reason: 'code_not_valid_for_target', + assignmentIndex: index, + }); + regionConflict = true; + continue; + } + unresolved.push({ - spoken, - reason: 'code_not_valid_for_target', + spoken: spokenOf(assignment) || classification.code, + reason: 'prosthesis_type_ambiguous', + candidates: [...leaves].sort(), assignmentIndex: index, }); continue; } - if (resolution.kind === 'tooth') { - targets.add(resolution.fdi); - } else { - for (const sentinel of archSentinels(resolution.arch)) - targets.add(sentinel); + if (typeof rawCode === 'string' && rawCode.trim()) { + unresolved.push({ + spoken: rawCode, + reason: 'unknown_catalog_code', + assignmentIndex: index, + }); } } + const targets = new Set(); + + for (const target of resolvedTargets) { + // EVERY named leaf must suit the target, not merely one of them. `some` let a legal + // crown carry an arch-only appliance onto the same tooth. + if (regionConflict) continue; + const accepted = acceptableRegions(target.kind); + const valid = [...leafRegions].every((region) => accepted.has(region)); + if (!valid) { + unresolved.push({ + spoken: target.spoken, + reason: 'code_not_valid_for_target', + assignmentIndex: index, + }); + continue; + } + for (const code of target.codes) targets.add(code); + } + return { targets: [...targets], types: [...leafCodes], diff --git a/frontend/src/components/ui/treatment/VoiceReviewSheet.tsx b/frontend/src/components/ui/treatment/VoiceReviewSheet.tsx index 072263f..768f020 100644 --- a/frontend/src/components/ui/treatment/VoiceReviewSheet.tsx +++ b/frontend/src/components/ui/treatment/VoiceReviewSheet.tsx @@ -116,8 +116,15 @@ export function VoiceReviewSheet({ const typeForcedByProsthesis = available.prosthesis && selection.prosthesis && effective.treatmentType != null; + // What Apply actually sends. The locked type row is ticked on screen, so it has to be + // ticked in the count and in the payload alike — counting the raw `selection` showed + // "Apply 1" while two fields landed. + const effectiveSelection: VoiceApplySelection = typeForcedByProsthesis + ? { ...selection, treatmentType: true } + : selection; + const nothingToApply = !hasAnythingToApply(effective, labDependentCodes); - const selectedCount = countSelected(selection, available); + const selectedCount = countSelected(effectiveSelection, available); const targetLabel = (target: string): string => { if (target === ARCH_TOOTH_UPPER) return t('upperArch'); @@ -394,13 +401,7 @@ export function VoiceReviewSheet({ type="button" variant="primary" disabled={selectedCount === 0} - onClick={() => - onApply( - // The locked type row is ticked in the UI, so it must be ticked in what applies. - typeForcedByProsthesis ? { ...selection, treatmentType: true } : selection, - effective, - ) - } + onClick={() => onApply(effectiveSelection, effective)} fullWidth className="sm:w-auto" >