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>
Two ways a voice failure described itself wrongly.
describe() built the quoted-back text from fields that are all nullable on
the wire, and toVoiceIntent casts rather than checks — so a half-classified
deadline rendered as “null null” — not a usable date, and an offset with no
amount as “+NaN day”. Blank is already handled by the sheet; it now falls
back to that.
The DTO's constraints resolved to unrelated codes: maxLength fell through
to VALIDATION_FIELD_REQUIRED, so an oversized recording said a field was
missing, and isIn maps to VALIDATION_LANGUAGE_INVALID, so an unsupported
container said the language was invalid. Both now name their own code —
the validation factory already returns a message verbatim when it is itself
a known ErrorCode, so this needs no change to the shared mapping.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The meter wrote React state from a requestAnimationFrame loop, and the hook
lives in TreatmentWorkspace — so every frame re-rendered the details editor,
the FDI chart, the lab panel and the history rail. About 7,200 whole-tree
renders across a two-minute recording, while the user is dictating.
Now samples every frame but publishes at LEVEL_POLL_MS, the rate the
elapsed timer already used. Peaks between publishes are carried forward, so
the meter stays responsive to transients rather than sampling at 10 Hz.
Also adds the catch the start path never had: new MediaRecorder() and
recorder.start() both throw on some browsers, and by then the stream is
live. The rejection went unhandled, the UI sat at 'idle' showing nothing,
and the browser's recording indicator stayed lit until unmount.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
applyVoiceResult put the lab draft in state and stopped there. Every other
path that creates a LabCaseDraft — handleContinueToLab, handleLabCasesChange
— immediately runs persistDraft + persistLabCases, and the autosave effect
only watches `details`. So applying a voice result carrying a lab, a due
date and a prosthesis map, then reloading, kept the detail and silently
dropped all three: the surviving detail made it look like the save worked.
applyVoiceResult moves below persistDraft/persistLabCases so it can call
them, and writes detailsRef itself before persisting — persistDraft reads
that ref, and setDetails has not rendered by the time the save runs. The
ref is already written imperatively elsewhere for the same reason.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
initialVoiceSelection deliberately never auto-ticks an incomplete
prosthesis map, because a detail with an untyped tooth cannot ship — it
fails at dispatch instead. Picking a candidate tooth walked straight
through that rule: the tick was seeded once, so a map that was complete at
extraction stayed ticked after a tooth with no prosthesis type joined it,
and Apply attached a map assertCompleteToothProsthesisMap rejects.
Recomputed on each pick, and only ever downwards — re-ticking is the
clinician's call, not a side effect of un-picking.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
An under-specified tooth was a dead end: the sheet said what was missing
and the clinician had to leave and hunt for it on the chart. The readings
are enumerable, so the review sheet now renders them as chips — the one
interactive part of an otherwise read-only confirmation step.
A pick is folded into the result by withChosenTeeth() rather than tracked
alongside it, so the rows, the mini chart, the prosthesis warning and
applyVoiceResult all keep reading a single VoiceExtractionResult and none
of them has to know the chips exist. It unions rather than toggles: a
candidate can coincidentally be a tooth the recording already produced, and
tapping it must not deselect that one.
Two things that would otherwise make the chips look functional while
applying nothing: the teeth row is ticked on the first pick (it starts
unticked when the recording produced no teeth of its own), and the apply
count is now intersected with row availability so it cannot promise to
apply a row with nothing in it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
"ترمیم برای دندون دو" set the treatment type but reported the tooth as
unreadable. Nothing was misheard: position 2 arrived intact, with no
quadrant, because none was spoken — four teeth carry position 2 and the
resolver correctly refused to pick one. Only the label was wrong, and it
sent the clinician looking for a transcription fault.
Adds a tooth_missing_quadrant reason that names what is missing and shows
how to say it ("دو بالا راست"), and tells the model explicitly to report a
quadrant-less number with arch and side null rather than guessing.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Makes the feature reachable end to end: availability is fetched alongside the
catalogs, the capture hook drives the segmented control, and confirming the
review sheet appends a new detail.
Confirm always appends — it never edits an existing detail and never calls
onAddDetail. Ticked rows land on top of the seeded defaults, so unticking the
type row leaves the appointment-purpose default rather than a blank. Lab-side
rows ride on a lab case draft keyed by the detail's *client* id, so a brand-new
unsaved detail can carry a lab, due date and per-tooth prosthesis map.
Availability comes from the API rather than a NEXT_PUBLIC_* var, since those are
baked in at build time; a failure fetching it degrades to no microphone rather
than taking the treatment tab down.
From review of this commit:
- Unticking "teeth" while leaving "prosthesis" ticked attached prosthesis rows
for teeth the detail does not contain. Nothing downstream filters them —
assertCompleteToothProsthesisMap only checks detail-teeth ⊆ map, never the
reverse — so they would have reached task generation as lab work for teeth
nobody is treating. The map is now filtered to the detail's own teeth.
- The microphone was gated on the URL locale while the server resolved
everything from req.user.language. Those diverge (a bookmarked /fa/ URL, a
language toggle whose save failed), which would transcribe Persian with an
English hint and anchor "next Thursday" to a Monday week instead of a Saturday
one — or 403 from a visibly-enabled button. The client now sends the locale the
microphone was offered in, so the gate and the request agree by construction.
Also fixed from the previous review: a civil YYYY-MM-DD date rendered a day
early west of Greenwich (parsed as UTC midnight); the missing-teeth list
hardcoded the Arabic comma for all locales; and voiceApply had no ICU plural, so
the common single-field case read "Apply 1 fields".
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The microphone becomes the second segment of the Add detail button, built like
the detail chip's trash affordance in the same file — an overflow-hidden rounded
wrapper holding two raw <button>s divided by border-s — rather than two shared
Buttons, which each hardcode their own rounding and would fight a segmented
control. border-s puts the mic 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 behaviour.
The control never changes size while recording; the timer and level meter live
in a bar between the header row and the chip strip, because the header is
sm:justify-between and growing the button would shove the row on every start and
stop. The meter exists to prove the microphone is actually hearing something —
silence and a dead mic look identical otherwise.
Voice reaches the editor as one optional `voice` prop, so its absence *is* the
unavailable state and the two cannot disagree.
Fixes from review of this commit:
- mountedRef was set false on unmount and never re-armed, so under StrictMode
the hook was permanently "unmounted" in dev and recording silently never
started.
- onStart guarded only on `phase`, which does not change until getUserMedia
resolves; a second click during the permission prompt orphaned the first
MediaStream, leaving the mic indicator lit.
- Week start is now per locale. "Next Thursday" is week-relative, and hardcoding
Saturday put an en/nl clinician's deadline a week out.
- A missing `which` on a weekday intent is read as "this" rather than failing —
a bare weekday carries no qualifier, and rejecting it discarded a real
deadline.
- durationMs is client-reported and so is a claim, not enforcement; the cap is
now also checked against the vendor's own usage.seconds.
- Blob type falls back to the recorder's actual mimeType before webm, so old
Safari's mp4/aac clips are not mislabelled.
Two review findings were rejected as incorrect, both re-verified against live
sources: google/gemini-3.7-flash does exist on OpenRouter (1M context,
$0.375/$1.875 per M), and base64 JSON input_audio is the documented primary
path for /audio/transcriptions, with multipart as the OpenAI-compatible
alternative. The spec's stale "unverified" note is corrected, and the provider
now has unit tests covering the request shape, usage parsing, and that a vendor
error body never reaches the thrown message.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
MediaRecorder handling and the API call live in lib/, not in ui/, so
TreatmentDetailsEditor can stay presentational and take only a `voice` prop.
Container choice is made at record time and needs no transcode: Chrome and
Android give webm/opus, Safari and iPad give mp4/aac, and the transcription
endpoint accepts both. Safari's `audio/mp4` is sent as `m4a`, the name the
vendor's container list actually uses, so iPad recordings do not fail while
Chrome works. Older Safari shipped MediaRecorder without isTypeSupported, so
that path lets the browser choose rather than refusing outright.
From review of this commit:
- The auto-stop at maxMs guaranteed a 413. The client measures the final length
after the recorder has stopped, so a recording that runs to the cap always
reports slightly over it, and the server rejected exactly the recording the
auto-stop existed to save. The server now allows a documented 2s tolerance and
the client keeps reporting the true length, so telemetry stays honest.
- getUserMedia is async, so a permission granted after unmount installed a live
stream the cleanup effect had already run past — leaving the browser's
recording indicator lit with nothing listening. Guarded with a mounted ref.
- Client-side failures are now ApiError-shaped ({code, statusCode}) rather than
bare Errors, because getUserFacingError only resolves that shape; without it
errors.VOICE_MIC_DENIED was dead in all three locales.
Cancelling aborts the request, which closes the connection and aborts the
metered vendor call server-side rather than letting it settle unseen. The level
meter is best-effort: a blocked AudioContext costs the meter, not the recording.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
POST /voice/extract behind JwtAuthGuard + ClinicOrgGuard, plus
GET /voice/availability so the frontend can decide whether to render the
microphone — it cannot learn that from NEXT_PUBLIC_*, which are baked in at
build time.
Audio is held in memory for the request only: never written to disk, never a
Prisma row. The transcript goes back to the client and is not persisted. What
is logged is structured and patient-free — clip length, which fields resolved,
unresolved count, vendor cost, outcome — with log lines as the interim sink
until this repo has metrics infrastructure.
On extraction failure the transcript still travels back in the error details,
so the words the clinician already paid for can be salvaged into a note.
v1 ships ungated beyond a configured locale profile; the Plan.features design
is deferred, not dropped.
From review of this commit, four of which were load-bearing:
- Express's 100 kb default body limit rejected any recording past ~20 seconds,
making the endpoint unusable at its own 2-minute cap. Body parsers are now
registered explicitly with a 10 MB limit scoped to the voice route only.
Verified empirically: 600 KB reaches /api/voice/extract, while /api/auth/login
still 413s.
- ThrottlerGuard keys on req.ip, so behind nginx the whole deployment would
share one bucket and an abuser rotating IPs would bypass it. VoiceThrottlerGuard
keys on the user id instead — with no plan gate, this is the only control on
metered vendor spend.
- ThrottlerException had no 429 fallback and surfaced as INTERNAL_ERROR; the
guard now throws VOICE_RATE_LIMITED directly.
- durationMs was optional, so omitting it bypassed VOICE_MAX_RECORDING_MS
entirely. It is required.
- VOICE_UNSUPPORTED_FORMAT was dead code — the DTO's @IsIn already rejects
unknown containers — so it is gone rather than left unreachable.
ThrottlerModule is deliberately not bound as a global APP_GUARD: a global
ThrottlerGuard rate-limits every route against every named throttler, which
would have capped the whole API at the voice limit.
All seven remaining VOICE_* codes have errors.* keys in en, fa and nl.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Logical API errors throw stable codes so users see translated messages instead of a generic bad request.
Co-authored-by: Cursor <cursoragent@cursor.com>
