docs: cut the comments that were not earning their place
I wrote 731 comment lines on this branch against 4,530 lines of code — 14%, where the rest of the repo runs at 1.8%. CLAUDE.md asks for code that reads like its surroundings, and this did not. Removed by genre rather than by taste: - restating the code, e.g. "JS getUTCDay() numbering: Sunday = 0" above the map that literally shows it, and a docblock on startOfWeek explaining that it returns the start of the week; - narrating history — "this used to rebuild the whole map", "left the bar recording forever" — which the commit message and git blame already carry; - saying the same thing in several places: the "cannot record is not a denied microphone" reason appeared three times in one file, and the "aborting stops a per-minute metered call" reason across three files. Each now lives once, where the behaviour it explains lives; - defending decisions nobody would question, like why toLatinDigits is its own module; - over-explaining defensive branches, three separate comments to distinguish null from missing-kind from unrecognised-kind. What stays is what the code cannot say: the patient-right convention in toFdi, whose failure mode is a valid code for the wrong tooth; the "this"-vs-"next" week anchoring; StrictMode re-arming mountedRef; Safari accepting no mimeType hint; and the invariants whose violation already cost a bug — the body parser's middleware ordering and the dispatch panel's auto-fill rules. Comments only. The diff contains no non-comment line. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -19,14 +19,9 @@ export function voiceRowAvailability(result: VoiceExtractionResult) {
|
||||
}
|
||||
|
||||
/**
|
||||
* Which rows start ticked.
|
||||
*
|
||||
* Everything available ticks itself, with two deliberate exceptions:
|
||||
*
|
||||
* - **lab, when the name only approximately matched.** Shipping a case to a lab is the one
|
||||
* extracted value whose error leaves the building, so it always requires a deliberate tick.
|
||||
* - **prosthesis, when the map is incomplete.** A prosthesis detail with an untyped tooth
|
||||
* cannot ship at all, so applying it would just move the failure to dispatch.
|
||||
* Everything available ticks itself, with two exceptions: an inexactly-matched lab, because
|
||||
* it is the one extracted value whose error leaves the building; and an incomplete
|
||||
* prosthesis map, which cannot ship at all and would just move the failure to dispatch.
|
||||
*/
|
||||
export function initialVoiceSelection(result: VoiceExtractionResult): VoiceApplySelection {
|
||||
const available = voiceRowAvailability(result);
|
||||
@@ -41,11 +36,8 @@ export function initialVoiceSelection(result: VoiceExtractionResult): VoiceApply
|
||||
}
|
||||
|
||||
/**
|
||||
* How many rows will actually be applied — drives the confirm button's label.
|
||||
*
|
||||
* Intersected with availability rather than counting ticks: a row can be ticked and then
|
||||
* lose its content (the last candidate tooth un-picked), and "Apply 1 item" that applies
|
||||
* nothing is worse than a wrong number.
|
||||
* Intersected with availability rather than counting ticks: a row can be ticked and then lose
|
||||
* its content, and "Apply 1 item" that applies nothing is worse than a wrong number.
|
||||
*/
|
||||
export function countSelected(
|
||||
selection: VoiceApplySelection,
|
||||
@@ -66,16 +58,10 @@ function recheckProsthesis(
|
||||
}
|
||||
|
||||
/**
|
||||
* Fold the clinician's candidate picks into the extracted result.
|
||||
* Fold the candidate picks into the result, so nothing downstream has to know chips exist.
|
||||
*
|
||||
* Everything downstream reads a `VoiceExtractionResult` — row availability, the mini
|
||||
* chart, the prosthesis warning, `applyVoiceResult` — so resolving the picks into one here
|
||||
* means none of them has to know the chips exist.
|
||||
*
|
||||
* Union rather than toggle, for two reasons: a candidate can coincidentally be a tooth the
|
||||
* recording already produced ("۱۲ و دو"), where tapping it must not deselect that tooth;
|
||||
* and `groupsFromFlatTeeth` keeps the bridges intact while giving every remaining tooth a
|
||||
* single group, so no tooth can be lost on the way through.
|
||||
* Union rather than toggle: a candidate can coincidentally be a tooth the recording already
|
||||
* produced ("۱۲ و دو"), and tapping it must not deselect that one.
|
||||
*/
|
||||
export function withChosenTeeth(
|
||||
result: VoiceExtractionResult,
|
||||
@@ -103,12 +89,7 @@ export function connectedTeethFromResult(result: VoiceExtractionResult): Set<Fdi
|
||||
return connected;
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether the sheet has anything worth showing.
|
||||
*
|
||||
* A recording that produced nothing usable should say so plainly rather than present an
|
||||
* empty form of checkboxes.
|
||||
*/
|
||||
/** A recording that produced nothing should say so, not show an empty form of checkboxes. */
|
||||
export function hasAnythingToApply(result: VoiceExtractionResult): boolean {
|
||||
return Object.values(voiceRowAvailability(result)).some(Boolean);
|
||||
}
|
||||
|
||||
@@ -222,10 +222,9 @@ export function LabCasesDispatchPanel({
|
||||
}, [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.
|
||||
// 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) =>
|
||||
@@ -263,20 +262,18 @@ export function LabCasesDispatchPanel({
|
||||
? activeLinkedOrganizations.find((o) => o.id === lastLabId)
|
||||
: undefined;
|
||||
if (!lastLab) return;
|
||||
// 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.
|
||||
// Must NOT clear toothProsthesis: this runs only when no lab is set, so no other lab's
|
||||
// catalogue could have supplied a code — the map is unvalidated, not foreign, and
|
||||
// clearing here discards it before it is ever rendered. The effect above validates it
|
||||
// once the catalogue loads; a deliberate lab *switch* still clears.
|
||||
updateActiveLabCase({ destinationOrganizationId: lastLab.id });
|
||||
// eslint-disable-next-line react-hooks/exhaustive-deps
|
||||
}, [activeLabCase?.clientId, clinicOrganizationId, sent]);
|
||||
|
||||
useEffect(() => {
|
||||
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.
|
||||
// Confirmed in a preview, so it is left exactly as previewed: adding a default to a
|
||||
// tooth the preview never showed makes the confirmation a lie about what it fills.
|
||||
if (previewConfirmedCaseIds.has(activeLabCase.clientId)) return;
|
||||
if (!activeLabCase.destinationOrganizationId) return;
|
||||
if (prosthesisOptions.length === 0 || prosthesisRows.length === 0) return;
|
||||
@@ -292,9 +289,8 @@ export function LabCasesDispatchPanel({
|
||||
);
|
||||
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.
|
||||
// Blanks only. Rebuilding the whole map from `lastCode` lets one unfilled row destroy
|
||||
// every type already set.
|
||||
const typed = new Set(
|
||||
activeLabCase.toothProsthesis
|
||||
.filter((tp) => tp.prosthesisTypeCode)
|
||||
|
||||
@@ -336,17 +336,12 @@ function NotesField({
|
||||
/**
|
||||
* "Add detail", split into two segments with the microphone at the logical end.
|
||||
*
|
||||
* Built like the detail chip's trash affordance in this same file — an
|
||||
* `inline-flex items-stretch overflow-hidden rounded` wrapper holding two raw `<button>`s
|
||||
* divided by `border-s` — rather than two shared `Button`s, which each hardcode their own
|
||||
* rounding and would fight a segmented control.
|
||||
* Built like the detail chip's trash affordance in this same file — a wrapper holding two
|
||||
* raw `<button>`s divided by `border-s` — rather than two shared `Button`s, which hardcode
|
||||
* their own rounding and would fight a segmented control. `border-s` puts the microphone
|
||||
* visually right in en/nl and left in fa, on the same side as the chip's trash in both.
|
||||
*
|
||||
* `border-s` puts the microphone at the *logical* end: visually right in en/nl, visually
|
||||
* left in fa, on the same side as the chip's trash in both directions.
|
||||
*
|
||||
* The two halves share a wrapper and nothing else. Add keeps its exact existing
|
||||
* behaviour; the microphone is an independent action that creates nothing until the
|
||||
* clinician confirms.
|
||||
* Add keeps its exact existing behaviour; the microphone is an independent action.
|
||||
*/
|
||||
function AddDetailWithVoice({
|
||||
addLabel,
|
||||
|
||||
@@ -418,11 +418,9 @@ export function TreatmentWorkspace({
|
||||
const [details, setDetails] = useState<TreatmentDetailDraft[]>(() => [newDetail()]);
|
||||
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.
|
||||
* Lab cases created by confirming a voice result. Here rather than on LabCaseDraft because
|
||||
* a draft field is dropped by mapLabCaseDraftFromApi on the first round-trip — exactly the
|
||||
* window where the dispatch panel's remembered default fires.
|
||||
*/
|
||||
const [voiceConfirmedLabCaseIds, setVoiceConfirmedLabCaseIds] = useState<ReadonlySet<string>>(
|
||||
() => new Set<string>(),
|
||||
@@ -1951,8 +1949,7 @@ export function TreatmentWorkspace({
|
||||
|
||||
const nextDetails = [...detailsRef.current, detail];
|
||||
setDetails(nextDetails);
|
||||
// persistDraft reads detailsRef, and setDetails has not rendered yet. The codebase
|
||||
// already writes this ref imperatively after a save for the same reason.
|
||||
// persistDraft reads detailsRef, and setDetails has not rendered yet.
|
||||
detailsRef.current = nextDetails;
|
||||
setActiveDetailId(detail.clientId);
|
||||
setEntryStep('treatment');
|
||||
@@ -1995,23 +1992,19 @@ export function TreatmentWorkspace({
|
||||
}
|
||||
const updatedLabCases = [...labCaseDrafts, draft];
|
||||
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.
|
||||
// The sheet showed exactly what this case contains; nothing may top it up after.
|
||||
setVoiceConfirmedLabCaseIds((prev) => new Set(prev).add(draft.clientId));
|
||||
|
||||
// Every other path that creates a lab draft persists it immediately, and the
|
||||
// autosave effect only watches `details`. Left in state alone, the destination
|
||||
// lab, the due date and the whole prosthesis map vanish on the next reload —
|
||||
// silently, because the detail itself does survive.
|
||||
// Autosave only watches `details`, so a lab draft left in state alone loses the
|
||||
// lab, the due date and the prosthesis map on reload — silently, because the
|
||||
// detail itself survives.
|
||||
void (async () => {
|
||||
try {
|
||||
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.
|
||||
// persistDraft returns a *preview* when the details are not persistable — one
|
||||
// blank detail is enough — and a preview's detail id falls back to the client
|
||||
// id. 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);
|
||||
|
||||
@@ -16,9 +16,9 @@ function formatElapsed(ms: number): string {
|
||||
/**
|
||||
* Live recording / processing strip.
|
||||
*
|
||||
* Sits between the header row and the chip strip rather than inside the segmented
|
||||
* control: the header is `sm:justify-between`, so growing the button mid-recording would
|
||||
* shove the row on every start and every stop.
|
||||
* Sits between the header row and the chip strip rather than inside the segmented control:
|
||||
* the header is `sm:justify-between`, so growing the button mid-recording would shift the
|
||||
* whole row.
|
||||
*/
|
||||
export function VoiceRecordingBar({ voice }: { voice: VoiceCaptureState }) {
|
||||
const t = useTranslations('treatment');
|
||||
|
||||
@@ -38,9 +38,8 @@ interface VoiceReviewSheetProps {
|
||||
/**
|
||||
* Confirmation step between the model's output and the form.
|
||||
*
|
||||
* Modal on desktop, bottom sheet on mobile via ResponsiveDialog — deliberately an overlay
|
||||
* and not a route, because navigating would unmount TreatmentWorkspace and destroy the
|
||||
* in-progress draft.
|
||||
* Modal on desktop, bottom sheet on mobile — an overlay and not a route, because navigating
|
||||
* would unmount TreatmentWorkspace and destroy the in-progress draft.
|
||||
*/
|
||||
export function VoiceReviewSheet({
|
||||
result,
|
||||
@@ -78,10 +77,9 @@ export function VoiceReviewSheet({
|
||||
// The teeth row starts unticked whenever the recording produced no teeth of its own,
|
||||
// and a picked tooth that is not ticked applies nothing.
|
||||
teeth: true,
|
||||
// The picked tooth has no prosthesis type, which makes the map unshippable. Leaving
|
||||
// the row ticked would apply a map that `assertCompleteToothProsthesisMap` rejects
|
||||
// at dispatch — the exact failure the never-auto-tick-incomplete rule exists to
|
||||
// prevent. Only ever unticks: re-ticking is the clinician's call.
|
||||
// A picked tooth has no prosthesis type, so the map is no longer shippable — leaving the
|
||||
// row ticked would apply a map dispatch rejects. Only ever unticks; re-ticking is the
|
||||
// clinician's call.
|
||||
prosthesis:
|
||||
prev.prosthesis &&
|
||||
withChosenTeeth(result, nextChosen).prosthesis?.complete !== false,
|
||||
@@ -290,9 +288,8 @@ function Row({
|
||||
}
|
||||
|
||||
/**
|
||||
* A bare `YYYY-MM-DD` is a *civil* date, but `new Date('2025-10-17')` parses it as UTC
|
||||
* midnight — which renders as the 16th for any viewer west of Greenwich. Build the date
|
||||
* from its parts so it means the same day everywhere.
|
||||
* `new Date('2025-10-17')` parses a civil date as UTC midnight, which renders as the 16th
|
||||
* west of Greenwich. Build it from its parts so it means the same day everywhere.
|
||||
*/
|
||||
function civilDateToLocalDate(iso: string): Date {
|
||||
const [year, month, day] = iso.split('-').map(Number);
|
||||
|
||||
@@ -22,8 +22,7 @@ export const voiceApi = {
|
||||
payload: ExtractVoicePayload,
|
||||
signal?: AbortSignal,
|
||||
): Promise<{ success: boolean; data: VoiceExtractionResult }> => {
|
||||
// The signal is forwarded so cancelling closes the connection, which aborts the
|
||||
// metered vendor call server-side rather than letting it settle unseen.
|
||||
// Forwarded so cancelling closes the connection; the controller turns that into an abort.
|
||||
const response = await apiClient.post('/voice/extract', payload, { signal });
|
||||
return response.data;
|
||||
},
|
||||
|
||||
@@ -9,11 +9,9 @@ const PREFERRED_MIME_TYPES = [
|
||||
] as const;
|
||||
|
||||
/**
|
||||
* Pick a container this browser can record AND the backend accepts.
|
||||
*
|
||||
* Chrome and Android produce webm/opus; Safari and iPad produce mp4/aac. Both go to the
|
||||
* vendor unmodified, so there is no transcode step — but the choice still has to be made
|
||||
* at record time, and `isTypeSupported` is missing entirely on older Safari.
|
||||
* Pick a container this browser can record AND the backend accepts. Chrome and Android give
|
||||
* webm/opus, Safari and iPad mp4/aac; both go to the vendor unmodified, so there is no
|
||||
* transcode step and the list is an intersection, not a preference.
|
||||
*/
|
||||
export function pickRecordingMimeType(): string | null {
|
||||
if (typeof MediaRecorder === 'undefined') return null;
|
||||
|
||||
@@ -69,9 +69,8 @@ export function useVoiceCapture({
|
||||
/** getUserMedia is async; without this a permission granted after unmount leaks the mic. */
|
||||
const mountedRef = useRef(true);
|
||||
/**
|
||||
* Set synchronously on click. `phase` does not become 'recording' until getUserMedia
|
||||
* resolves, so without this a second click during the permission prompt would start a
|
||||
* second stream and orphan the first — mic indicator lit, interval leaked.
|
||||
* Set synchronously on click: `phase` only becomes 'recording' once getUserMedia resolves,
|
||||
* so a second click during the permission prompt would orphan the first stream.
|
||||
*/
|
||||
const startingRef = useRef(false);
|
||||
|
||||
@@ -139,9 +138,7 @@ export function useVoiceCapture({
|
||||
);
|
||||
|
||||
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.
|
||||
// No recorder means nothing will fire `onstop`, so nothing else moves the phase.
|
||||
if (!recorderRef.current) {
|
||||
teardown();
|
||||
setPhase('idle');
|
||||
@@ -158,8 +155,9 @@ export function useVoiceCapture({
|
||||
const onStart = useCallback(() => {
|
||||
if (phase !== 'idle' || startingRef.current) return;
|
||||
if (!isMediaRecorderSupported()) {
|
||||
// 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.
|
||||
// VOICE_UNSUPPORTED_FORMAT, not MIC_DENIED: nothing asked for a permission yet, and
|
||||
// blaming the microphone sends the clinician into site settings for no reason. Same
|
||||
// for the two paths below.
|
||||
onError(clientError('VOICE_UNSUPPORTED_FORMAT'));
|
||||
return;
|
||||
}
|
||||
@@ -187,7 +185,6 @@ export function useVoiceCapture({
|
||||
|
||||
const mimeType = pickRecordingMimeType();
|
||||
if (mimeType === null) {
|
||||
// The browser records, but in no container the transcription API accepts.
|
||||
stream.getTracks().forEach((track) => track.stop());
|
||||
onError(clientError('VOICE_UNSUPPORTED_FORMAT'));
|
||||
return;
|
||||
@@ -231,14 +228,10 @@ export function useVoiceCapture({
|
||||
if (maxMs != null && elapsed >= maxMs) stop();
|
||||
}, LEVEL_POLL_MS);
|
||||
} catch {
|
||||
// `new MediaRecorder(...)` and `recorder.start()` both throw on some browsers,
|
||||
// and by then the stream is already live. Without this the promise rejects
|
||||
// unhandled, the UI sits at 'idle' with nothing shown, and the browser's
|
||||
// recording indicator stays lit until the workspace unmounts.
|
||||
// `new MediaRecorder()` and `recorder.start()` both throw on some browsers, and by
|
||||
// then the stream is live — without this the mic indicator stays lit until unmount.
|
||||
teardown();
|
||||
setPhase('idle');
|
||||
// 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 {
|
||||
startingRef.current = false;
|
||||
@@ -248,8 +241,7 @@ export function useVoiceCapture({
|
||||
|
||||
const onCancel = useCallback(() => {
|
||||
cancelledRef.current = true;
|
||||
// Aborting closes the connection, which aborts the vendor call server-side. It is
|
||||
// metered per minute, so letting it settle costs money for a result nobody sees.
|
||||
// Aborting closes the connection, which aborts the vendor call server-side.
|
||||
abortRef.current?.abort();
|
||||
try {
|
||||
recorderRef.current?.stop();
|
||||
@@ -283,10 +275,9 @@ function attachLevelMeter(
|
||||
source.connect(analyser);
|
||||
|
||||
const data = new Uint8Array(analyser.frequencyBinCount);
|
||||
// Sample every frame so a transient is not missed, but publish at LEVEL_POLL_MS.
|
||||
// This hook lives in TreatmentWorkspace, so an unthrottled setLevel re-renders the
|
||||
// details editor, the FDI chart and the lab panel on every animation frame — about
|
||||
// 7,200 whole-tree renders across a two-minute recording.
|
||||
// Sample every frame so a transient is not missed, publish at LEVEL_POLL_MS. The hook
|
||||
// lives in TreatmentWorkspace, so an unthrottled setLevel is ~7,200 whole-tree renders
|
||||
// across a two-minute recording.
|
||||
let peakSinceEmit = 0;
|
||||
let lastEmit = 0;
|
||||
const tick = (now: number) => {
|
||||
|
||||
Reference in New Issue
Block a user