Compare commits

...

2 Commits

Author SHA1 Message Date
9ec23d0a58 fix: stop voice entry posting an unsaved detail id, and name failures right
Review findings on this branch.

The lab-case save could be posted against a detail the server has never
seen. persistDraft returns a *preview* treatment instead of saving when any
detail lacks a treatment type — the blank one the workspace opens with is
enough — and a preview's detail id falls back to the client id. Recording
straight after opening a visit and confirming a result with a lab or due
date would send that id and fail the whole save. It now checks what came
back rather than the precondition, so it holds for every early return
persistDraft has.

stop() optional-chained into a no-op when the recorder was already gone,
leaving the bar recording forever with a live timer and only Cancel as a
way out.

Three "this browser cannot record" paths reported VOICE_MIC_DENIED — no
MediaRecorder at all, no container the API accepts, and a recorder that
throws after permission was already granted. Telling clinicians their
microphone was denied sends them hunting for a permission nothing asked
for; they now report VOICE_UNSUPPORTED_FORMAT.

The voice route's large-body match stripped every trailing slash while
Express ignores exactly one, so '/api/voice/extract//' bought a 10 MB
buffer for a request that then 404s.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-21 17:44:51 +08:00
5d8393c1eb fix(frontend): stop the remembered prosthesis default rewriting the map
Dictating "12 روکش PFM, 13 روکش PFZ" previewed correctly and then landed in
the form as PFM on both teeth.

The stored data was never wrong — the dev database holds 12 → pfm_crown and
13 → pfz_crown with selectionGroupIds matching the detail's groups exactly,
and a page reload renders it correctly. The damage was live client state:
the dispatch panel's "last type used for this lab" default rebuilt the
*entire* map from one code, so a single row reading as unfilled destroyed
every type already set.

Two changes:

- It fills blanks now, and leaves every entry that already carries a type
  alone. The bulk "apply to all" select only pre-sets itself when the fill
  really did cover every tooth, instead of claiming one type while the rows
  below disagree.
- A lab case created by confirming a voice result is exempt from the
  default entirely. The review sheet is a contract: topping the case up
  with a type for a tooth the preview never showed makes the confirmation
  step a lie about what it was going to fill.

The exemption is tracked in workspace state rather than on LabCaseDraft
because a draft field is dropped by mapLabCaseDraftFromApi on the first
server round-trip — exactly the window this failure lives in.

isProsthesisMapComplete is deliberately untouched: its strict
selectionGroupId match succeeds on the real data, so loosening it would
have been a blind change to a working path.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-21 17:16:46 +08:00
6 changed files with 119 additions and 8 deletions

View File

@@ -93,6 +93,9 @@ describe('createJsonBodyParser', () => {
'/api/voice/extract/extra', '/api/voice/extract/extra',
'/api/voice', '/api/voice',
'/voice/extract', '/voice/extract',
// Express ignores one trailing slash, not two — this one never routes, so it must
// not get the large parser either.
'/api/voice/extract//',
]) { ]) {
const res = await request(buildApp()).post(path).send(bodyOfKb(300)); const res = await request(buildApp()).post(path).send(bodyOfKb(300));
expect(res.status).toBe(413); expect(res.status).toBe(413);

View File

@@ -31,7 +31,10 @@ export const VOICE_BODY_LIMIT = '10mb';
* recording — a failure that looks like a broken microphone, not a routing detail. * recording — a failure that looks like a broken microphone, not a routing detail.
*/ */
function isVoiceExtractPath(path: string): boolean { function isVoiceExtractPath(path: string): boolean {
return path.toLowerCase().replace(/\/+$/, '') === VOICE_EXTRACT_PATH; // Exactly one trailing slash, because that is exactly what Express ignores. Stripping
// every trailing slash would hand the 10 MB parser to `/api/voice/extract//`, which
// buffers the body and then 404s — memory spent on a request that never routes.
return path.toLowerCase().replace(/\/$/, '') === VOICE_EXTRACT_PATH;
} }
export function createJsonBodyParser(): RequestHandler { export function createJsonBodyParser(): RequestHandler {

View File

@@ -482,6 +482,12 @@ that justified this whole design.
- any row carrying an unresolved item or an incomplete prosthesis map. - any row carrying an unresolved item or an incomplete prosthesis map.
- Unresolved items are shown with what was heard ("دندان شیری — بازشناسی نشد"), so the - Unresolved items are shown with what was heard ("دندان شیری — بازشناسی نشد"), so the
clinician can see what the system did not understand. clinician can see what the system did not understand.
- **The sheet is a contract: confirm fills exactly what it previewed — no more.** Per-detail
conveniences that would top the case up afterwards are suppressed for a voice-created
case; concretely, the dispatch panel's remembered-prosthesis default
(`previewConfirmedCaseIds`). A default that quietly adds a prosthesis type to a tooth the
sheet never mentioned turns the confirmation step into a lie about what it was going to
do, which is the whole reason the step exists.
- An item that carries `candidates` renders them as **tappable chips** — the one place the - An item that carries `candidates` renders them as **tappable chips** — the one place the
sheet is interactive. Picking one folds the tooth into the result (`withChosenTeeth`) and sheet is interactive. Picking one folds the tooth into the result (`withChosenTeeth`) and
ticks the teeth row, so an under-specified tooth is one tap from resolved instead of a ticks the teeth row, so an under-specified tooth is one tap from resolved instead of a

View File

@@ -28,6 +28,9 @@ import type { PatientLabCaseSummary } from '@/types/lab-case-activity';
import { getUserFacingError } from '@/components/shared/formatApiError'; import { getUserFacingError } from '@/components/shared/formatApiError';
import { groupsFromFlatTeeth } from '@/components/treatment/toothSelectionGroups'; import { groupsFromFlatTeeth } from '@/components/treatment/toothSelectionGroups';
/** Stable empty default — a fresh Set literal would re-fire the auto-fill effect. */
const EMPTY_CASE_IDS: ReadonlySet<string> = new Set<string>();
interface LabCasesDispatchPanelProps { interface LabCasesDispatchPanelProps {
details: TreatmentDetailDraft[]; details: TreatmentDetailDraft[];
activeDetailId: string; activeDetailId: string;
@@ -41,6 +44,12 @@ interface LabCasesDispatchPanelProps {
onLabCaseMarkedRead?: (labCaseId: string) => void; onLabCaseMarkedRead?: (labCaseId: string) => void;
onLabCaseActivityChange?: () => void; onLabCaseActivityChange?: () => void;
activeLabCaseId: string | null; activeLabCaseId: string | null;
/**
* Lab cases whose contents the clinician already confirmed in a preview (voice entry).
* The remembered-prosthesis default is suppressed for these: the preview is a contract,
* and filling teeth it never showed would break it.
*/
previewConfirmedCaseIds?: ReadonlySet<string>;
onLabCasesChange: (labCases: LabCaseDraft[]) => void; onLabCasesChange: (labCases: LabCaseDraft[]) => void;
disabled: boolean; disabled: boolean;
canEdit: boolean; canEdit: boolean;
@@ -104,6 +113,11 @@ function isProsthesisMapComplete(
); );
} }
/** Identity of one prosthesis entry: a tooth on a detail. */
function toothKey(detailClientId: string, tooth: string): string {
return `${detailClientId}::${tooth}`;
}
function toothProsthesisForRows( function toothProsthesisForRows(
rows: ProsthesisGroupRow[], rows: ProsthesisGroupRow[],
prosthesisTypeCode: string, prosthesisTypeCode: string,
@@ -131,6 +145,7 @@ export function LabCasesDispatchPanel({
onLabCaseMarkedRead, onLabCaseMarkedRead,
onLabCaseActivityChange, onLabCaseActivityChange,
activeLabCaseId, activeLabCaseId,
previewConfirmedCaseIds = EMPTY_CASE_IDS,
onLabCasesChange, onLabCasesChange,
disabled, disabled,
canEdit, canEdit,
@@ -206,6 +221,21 @@ export function LabCasesDispatchPanel({
}; };
}, [activeLabCase?.destinationOrganizationId]); }, [activeLabCase?.destinationOrganizationId]);
useEffect(() => {
// The invariant the old wipe was protecting, enforced where it can actually be
// checked: no code survives that the destination lab does not offer. Guarded on a
// non-empty catalogue because the fetch above falls back to [] on failure, and a
// failed request must not erase the clinician's work.
if (!activeLabCase || sent || prosthesisOptions.length === 0) return;
const offered = new Set(prosthesisOptions.map((opt) => opt.code));
const kept = activeLabCase.toothProsthesis.filter((tp) =>
offered.has(tp.prosthesisTypeCode),
);
if (kept.length === activeLabCase.toothProsthesis.length) return;
updateActiveLabCase({ toothProsthesis: kept });
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [prosthesisOptions, activeLabCase?.clientId, sent]);
useEffect(() => { useEffect(() => {
setPendingComment(''); setPendingComment('');
}, [activeLabCase?.clientId]); }, [activeLabCase?.clientId]);
@@ -233,12 +263,21 @@ export function LabCasesDispatchPanel({
? activeLinkedOrganizations.find((o) => o.id === lastLabId) ? activeLinkedOrganizations.find((o) => o.id === lastLabId)
: undefined; : undefined;
if (!lastLab) return; if (!lastLab) return;
updateActiveLabCase({ destinationOrganizationId: lastLab.id, toothProsthesis: [] }); // Deliberately does NOT clear toothProsthesis. This runs only when no lab is set, so
// there is no other lab's catalogue for a code to have come from — the map is simply
// unvalidated, not foreign. Clearing it here threw away a whole voice-dictated
// prosthesis map before it was ever rendered. Validation happens against the
// catalogue below, once it loads; a deliberate lab *switch* still clears.
updateActiveLabCase({ destinationOrganizationId: lastLab.id });
// eslint-disable-next-line react-hooks/exhaustive-deps // eslint-disable-next-line react-hooks/exhaustive-deps
}, [activeLabCase?.clientId, clinicOrganizationId, sent]); }, [activeLabCase?.clientId, clinicOrganizationId, sent]);
useEffect(() => { useEffect(() => {
if (!activeLabCase || sent) return; if (!activeLabCase || sent) return;
// A case whose contents the clinician already confirmed in a preview is left exactly
// as previewed. Adding a remembered default to teeth the preview never mentioned
// would make the confirmation step a lie about what it was going to fill.
if (previewConfirmedCaseIds.has(activeLabCase.clientId)) return;
if (!activeLabCase.destinationOrganizationId) return; if (!activeLabCase.destinationOrganizationId) return;
if (prosthesisOptions.length === 0 || prosthesisRows.length === 0) return; if (prosthesisOptions.length === 0 || prosthesisRows.length === 0) return;
const fillKey = `${activeLabCase.clientId}:${prosthesisRows.length}`; const fillKey = `${activeLabCase.clientId}:${prosthesisRows.length}`;
@@ -252,14 +291,39 @@ export function LabCasesDispatchPanel({
activeLabCase.destinationOrganizationId, activeLabCase.destinationOrganizationId,
); );
if (!lastCode || !prosthesisOptions.some((opt) => opt.code === lastCode)) return; if (!lastCode || !prosthesisOptions.some((opt) => opt.code === lastCode)) return;
// Fill the blanks. This used to rebuild the whole map from `lastCode`, so one row
// reading as missing overwrote every type the clinician (or a dictation) had already
// set — a convenience default quietly destroying real choices.
const typed = new Set(
activeLabCase.toothProsthesis
.filter((tp) => tp.prosthesisTypeCode)
.map((tp) => toothKey(tp.detailClientId, tp.tooth)),
);
const blanks = toothProsthesisForRows(prosthesisRows, lastCode).filter(
(entry) => !typed.has(toothKey(entry.detailClientId, entry.tooth)),
);
if (blanks.length === 0) {
autoFilledCaseRef.current = fillKey; autoFilledCaseRef.current = fillKey;
setApplyAllProsthesis(lastCode); return;
updateActiveLabCase({ toothProsthesis: toothProsthesisForRows(prosthesisRows, lastCode) }); }
autoFilledCaseRef.current = fillKey;
// Only claim "all teeth" when the fill really did cover all of them; otherwise the
// bulk select shows one type while the rows below disagree with it.
if (blanks.length === flatToothCount) setApplyAllProsthesis(lastCode);
updateActiveLabCase({
toothProsthesis: [
...activeLabCase.toothProsthesis.filter((tp) => tp.prosthesisTypeCode),
...blanks,
],
});
// eslint-disable-next-line react-hooks/exhaustive-deps // eslint-disable-next-line react-hooks/exhaustive-deps
}, [ }, [
activeLabCase?.clientId, activeLabCase?.clientId,
activeLabCase?.destinationOrganizationId, activeLabCase?.destinationOrganizationId,
clinicOrganizationId, clinicOrganizationId,
previewConfirmedCaseIds,
prosthesisOptions, prosthesisOptions,
prosthesisRows.length, prosthesisRows.length,
sent, sent,

View File

@@ -417,6 +417,16 @@ export function TreatmentWorkspace({
const [details, setDetails] = useState<TreatmentDetailDraft[]>(() => [newDetail()]); const [details, setDetails] = useState<TreatmentDetailDraft[]>(() => [newDetail()]);
const [labCaseDrafts, setLabCaseDrafts] = useState<LabCaseDraft[]>([]); const [labCaseDrafts, setLabCaseDrafts] = useState<LabCaseDraft[]>([]);
/**
* Lab cases created by confirming a voice result.
*
* Kept here rather than as a field on LabCaseDraft on purpose: a draft field is dropped
* by mapLabCaseDraftFromApi on the first server round-trip, which is exactly the window
* where the dispatch panel's remembered-prosthesis default would fire.
*/
const [voiceConfirmedLabCaseIds, setVoiceConfirmedLabCaseIds] = useState<ReadonlySet<string>>(
() => new Set<string>(),
);
const [activeDetailId, setActiveDetailId] = useState<string>(() => details[0].clientId); const [activeDetailId, setActiveDetailId] = useState<string>(() => details[0].clientId);
const [activeLabCaseId, setActiveLabCaseId] = useState<string | null>(null); const [activeLabCaseId, setActiveLabCaseId] = useState<string | null>(null);
const [savedSnapshot, setSavedSnapshot] = useState<string | null>(null); const [savedSnapshot, setSavedSnapshot] = useState<string | null>(null);
@@ -1985,6 +1995,9 @@ export function TreatmentWorkspace({
} }
const updatedLabCases = [...labCaseDrafts, draft]; const updatedLabCases = [...labCaseDrafts, draft];
setLabCaseDrafts(updatedLabCases); setLabCaseDrafts(updatedLabCases);
// The sheet already showed the clinician exactly what this case would contain, so
// the dispatch panel must not top it up with a remembered default afterwards.
setVoiceConfirmedLabCaseIds((prev) => new Set(prev).add(draft.clientId));
// Every other path that creates a lab draft persists it immediately, and the // Every other path that creates a lab draft persists it immediately, and the
// autosave effect only watches `details`. Left in state alone, the destination // autosave effect only watches `details`. Left in state alone, the destination
@@ -1993,6 +2006,14 @@ export function TreatmentWorkspace({
void (async () => { void (async () => {
try { try {
const saved = await persistDraft({ force: true }); const saved = await persistDraft({ force: true });
// persistDraft returns a *preview* treatment rather than saving when the
// details are not persistable — one blank detail, the kind the workspace opens
// with, is enough. A preview's detail id falls back to the client id, so
// posting lab cases against it would send the server an id it has never seen
// and fail the whole save. Check what came back, not the precondition, so this
// holds for every early return persistDraft has.
const savedDetail = saved.details.find((d) => d.clientId === detail.clientId);
if (!savedDetail?.id || savedDetail.id === detail.clientId) return;
await persistLabCases(saved, updatedLabCases); await persistLabCases(saved, updatedLabCases);
} catch (error: unknown) { } catch (error: unknown) {
showError(getUserFacingError(error, tErrors, t('errorSaveLabShipments'))); showError(getUserFacingError(error, tErrors, t('errorSaveLabShipments')));
@@ -2745,6 +2766,7 @@ export function TreatmentWorkspace({
} }
}} }}
activeLabCaseId={activeLabCaseId} activeLabCaseId={activeLabCaseId}
previewConfirmedCaseIds={voiceConfirmedLabCaseIds}
onLabCasesChange={handleLabCasesChange} onLabCasesChange={handleLabCasesChange}
disabled={!canEditTreatmentForDay} disabled={!canEditTreatmentForDay}
canEdit={canEdit} canEdit={canEdit}

View File

@@ -139,8 +139,16 @@ export function useVoiceCapture({
); );
const stop = useCallback(() => { const stop = useCallback(() => {
// No recorder means nothing will fire `onstop`, so nothing else will move the phase.
// Optional-chaining into a no-op here left the bar recording forever with a running
// timer, and only Cancel could get out of it.
if (!recorderRef.current) {
teardown();
setPhase('idle');
return;
}
try { try {
recorderRef.current?.stop(); recorderRef.current.stop();
} catch { } catch {
teardown(); teardown();
setPhase('idle'); setPhase('idle');
@@ -150,7 +158,9 @@ export function useVoiceCapture({
const onStart = useCallback(() => { const onStart = useCallback(() => {
if (phase !== 'idle' || startingRef.current) return; if (phase !== 'idle' || startingRef.current) return;
if (!isMediaRecorderSupported()) { if (!isMediaRecorderSupported()) {
onError(clientError('VOICE_MIC_DENIED')); // Not a permission problem: this browser cannot record at all. Saying "microphone
// denied" sends the clinician to hunt for a permission nothing ever asked for.
onError(clientError('VOICE_UNSUPPORTED_FORMAT'));
return; return;
} }
@@ -177,8 +187,9 @@ export function useVoiceCapture({
const mimeType = pickRecordingMimeType(); const mimeType = pickRecordingMimeType();
if (mimeType === null) { if (mimeType === null) {
// The browser records, but in no container the transcription API accepts.
stream.getTracks().forEach((track) => track.stop()); stream.getTracks().forEach((track) => track.stop());
onError(clientError('VOICE_MIC_DENIED')); onError(clientError('VOICE_UNSUPPORTED_FORMAT'));
return; return;
} }
@@ -226,7 +237,9 @@ export function useVoiceCapture({
// recording indicator stays lit until the workspace unmounts. // recording indicator stays lit until the workspace unmounts.
teardown(); teardown();
setPhase('idle'); setPhase('idle');
onError(clientError('VOICE_MIC_DENIED')); // The permission was already granted by this point — what failed is the recorder
// itself, so this is "this browser cannot record", not "you denied the mic".
onError(clientError('VOICE_UNSUPPORTED_FORMAT'));
} finally { } finally {
startingRef.current = false; startingRef.current = false;
} }