Questionnaire v2: submit-path hardening and render-failure containment

src/components/Common/PluginErrorBoundary.tsx

@@ -1,14 +1,21 @@
 import React from "react";
 
+interface PluginErrorBoundaryProps {
+  children: React.ReactNode;
+  pluginName: string;
+  fallback?: React.ReactNode;
+  /** Notified once the boundary has caught. Callers that must react to the
+   *  failure elsewhere in the app use it — the questionnaire fill page
+   *  records the question so submit-time validation stops requiring an
+   *  input that is no longer on screen. */
+  onError?: (error: Error) => void;
+}
+
 export class PluginErrorBoundary extends React.Component<
-  { children: React.ReactNode; pluginName: string; fallback?: React.ReactNode },
+  PluginErrorBoundaryProps,
   { hasError: boolean }
 > {
-  constructor(props: {
-    children: React.ReactNode;
-    pluginName: string;
-    fallback?: React.ReactNode;
-  }) {
+  constructor(props: PluginErrorBoundaryProps) {
     super(props);
     this.state = { hasError: false };
   }
@@ -23,6 +30,7 @@ export class PluginErrorBoundary extends React.Component<
       error,
       errorInfo,
     );
+    this.props.onError?.(error);
   }
 
   render() {

src/components/QuestionnaireV2/fill/submit/composeBatch.ts

@@ -67,6 +67,13 @@ export interface ComposeBatchArgs {
   questionnaire: QuestionnaireRead;
   responses: Record<string, QuestionnaireResponse>;
   subject: FillSubject;
+  /** Question ids whose structured slot threw and shows the error
+   *  boundary's notice (`structuredRenderFailedAtom`). The validators skip
+   *  these on the premise that their data never submits — this is the
+   *  compose half of that bargain. Without it, rows recorded BEFORE the
+   *  component broke would post to the domain APIs with their type's
+   *  `validate` never run, from a section the UI presents as inert. */
+  renderFailed?: ReadonlySet<string>;
   /** Resuming a server draft — appends the completion PUT. */
   continueDraftId?: string;
 }
@@ -104,6 +111,7 @@ export async function composeBatch({
   questionnaire,
   responses,
   subject,
+  renderFailed,
   continueDraftId,
 }: ComposeBatchArgs): Promise<StructuredBatchEntry[]> {
   // Narrowed once, up front: the structured leg and the draft PUT both
@@ -129,6 +137,11 @@ export async function composeBatch({
       if (!response) continue;
 
       if (question.type === "structured" && question.structured_type) {
+        // The slot's component threw — the clinician sees a notice, not
+        // their data, and validateStructured skipped this question's
+        // `validate` for the same reason. What the UI shows as inert must
+        // not submit behind its back.
+        if (renderFailed?.has(question.id)) continue;
         // The recorded entries must belong to this question's type — the
         // guard `structuredDataOf` used to carry, kept now that the data
         // read is untyped.
@@ -165,11 +178,14 @@ export async function composeBatch({
         continue;
       }
 
-      if (
-        response.values.length > 0 &&
-        response.values[0]?.value !== "" &&
-        !response.structured_type
-      ) {
+      // Every plain leaf goes through serialization; the content decision
+      // is `serializeResponseValues`' alone (its filter keeps value-,
+      // coding- and unit-carrying entries). Gating here on values[0]
+      // dropped a repeats answer wholesale when its FIRST row was cleared
+      // in place — later rows silently never submitted, while the
+      // required check (which scans every entry) reported the question
+      // answered.
+      if (!response.structured_type) {
         answeredLeaves.push(response);
       }
     }

src/components/QuestionnaireV2/fill/submit/useSubmitQuestionnaire.ts

@@ -1,11 +1,12 @@
 import { useMutation } from "@tanstack/react-query";
-import { useCallback, useState } from "react";
+import { useCallback, useRef, useState } from "react";
 import { useTranslation } from "react-i18next";
 import { toast } from "sonner";
 
 import {
   errorsAtom,
   responsesAtom,
+  structuredRenderFailedAtom,
 } from "@/components/QuestionnaireV2/form/engine/store";
 import { collectRequiredErrors } from "@/components/QuestionnaireV2/form/validation";
 
@@ -52,7 +53,12 @@ function scrollToQuestion(questionId: string) {
       `[data-question-id="${questionId}"]`,
     );
     if (!block) return;
-    block.scrollIntoView({ block: "center", behavior: "smooth" });
+    block.scrollIntoView({
+      block: "center",
+      behavior: window.matchMedia("(prefers-reduced-motion: reduce)").matches
+        ? "auto"
+        : "smooth",
+    });
     const input = document.getElementById(`question-input-${questionId}`);
     if (input) {
       input.focus({ preventScroll: true });
@@ -148,35 +154,53 @@ export function useSubmitFillSession({
     },
   });
 
-  const submit = useCallback(async () => {
+  const runSubmit = useCallback(async () => {
     // 1) Validate every form against its own store; the first failure
     //    anywhere in the session decides where we scroll. The renderer's
-    //    flat subject view goes in so the required check can tell a
-    //    structured question that HAS an input from one showing a
-    //    placeholder (see form/validation.ts).
+    //    flat subject view and the form's render-failed set go in so the
+    //    required check can tell a structured question that HAS an input
+    //    from one showing a notice (see form/validation.ts).
     const rendererSubject = rendererSubjectOf(subject);
     let firstError: { formKey: string; questionId: string } | undefined;
     for (const form of forms) {
       const store = getStore(form.key);
       if (!store) continue;
       const responses = store.get(responsesAtom);
+      const renderFailed = store.get(structuredRenderFailedAtom);
       const clientErrors: QuestionValidationError[] = [
         ...collectRequiredErrors(form.questionnaire.questions, responses, t, {
           questionnaire: form.questionnaire,
           subject: rendererSubject,
+          renderFailed,
         }),
         ...collectStructuredErrors(
           form.questionnaire,
           responses,
           rendererSubject,
+          renderFailed,
           t,
         ),
       ];
       store.set(errorsAtom, clientErrors);
       if (clientErrors.length > 0 && !firstError) {
+        // "First" in the clinician's reading order, not in validator
+        // order — the array concatenates every required failure before
+        // any structured one, so its head can sit far below an earlier
+        // failing question on screen.
+        const failing = new Set(clientErrors.map((error) => error.question_id));
+        const inTreeOrder = (questions: Question[]): string | undefined => {
+          for (const question of questions) {
+            if (failing.has(question.id)) return question.id;
+            const found = inTreeOrder(question.questions ?? []);
+            if (found) return found;
+          }
+          return undefined;
+        };
         firstError = {
           formKey: form.key,
-          questionId: clientErrors[0].question_id,
+          questionId:
+            inTreeOrder(form.questionnaire.questions) ??
+            clientErrors[0].question_id,
         };
       }
     }
@@ -205,6 +229,7 @@ export function useSubmitFillSession({
               questionnaire: form.questionnaire,
               responses: store.get(responsesAtom),
               subject,
+              renderFailed: store.get(structuredRenderFailedAtom),
               continueDraftId: form.isPrimary ? continueDraftId : undefined,
             });
           }),
@@ -240,5 +265,39 @@ export function useSubmitFillSession({
     submitBatch({ requests });
   }, [forms, getStore, subject, continueDraftId, submitBatch, t]);
 
-  return { submit, isPending, serverErrors };
+  // Compose runs BEFORE the mutation starts, and structured
+  // `buildRequests` are async — during that window the mutation's
+  // isPending is still false, so a second click would validate and
+  // compose a second identical batch. The ref closes the window
+  // synchronously (state alone leaves the same-tick gap); the state twin
+  // keeps the button disabled for the same span.
+  const composingRef = useRef(false);
+  const [isComposing, setIsComposing] = useState(false);
+
+  /**
+   * The one entry point, and the outermost containment boundary. The page
+   * fires this as `onSubmit={() => void submit()}`, so ANY escaping
+   * rejection — a plugin component's getter, a malformed store record, a
+   * future call added inside `runSubmit` — would become an unhandled
+   * promise rejection and turn Save Changes into a silent no-op. Failing
+   * loudly is the floor: the clinician always learns the submission did
+   * not happen, and the original error still reaches the console for the
+   * developer.
+   */
+  const submit = useCallback(async () => {
+    if (composingRef.current || isPending) return;
+    composingRef.current = true;
+    setIsComposing(true);
+    try {
+      await runSubmit();
+    } catch (error) {
+      console.error("Questionnaire submission failed unexpectedly", error);
+      toast.error(t("questionnaire_submission_failed"));
+    } finally {
+      composingRef.current = false;
+      setIsComposing(false);
+    }
+  }, [runSubmit, isPending, t]);
+
+  return { submit, isPending: isPending || isComposing, serverErrors };
 }

src/components/QuestionnaireV2/fill/submit/validateStructured.ts

@@ -22,11 +22,12 @@ import type { QuestionnaireRead } from "@/types/questionnaire/questionnaire";
  * ones alike). Same disabled-subtree skip as composeBatch/validation.ts.
  *
  * A question whose slot cannot show an input is skipped — subject
- * mismatch, or a mount that can't supply an id the type `requires`. The
- * clinician has no way to answer those, and composeBatch drops their data
- * regardless, so validating them would only block submission on data that
- * never submits. The predicate is the one `StructuredSlot` renders from;
- * see `StructuredSlotState`'s parity note.
+ * mismatch, a mount that can't supply an id the type `requires`, or a
+ * component that threw and left the error boundary's notice in its place.
+ * The clinician has no way to answer any of those, and composeBatch drops
+ * their data regardless, so validating them would only block submission on
+ * data that never submits. Shares one predicate with `collectRequiredErrors`
+ * — see `structuredQuestionIsAnswerable`'s parity note.
  *
  * A type this deployment doesn't have blocks the submit only when the
  * question is required: an optional question whose plugin is disabled is
@@ -38,6 +39,7 @@ export function collectStructuredErrors(
   questionnaire: QuestionnaireRead,
   responses: Record<string, QuestionnaireResponse>,
   subject: RendererSubject,
+  renderFailed: ReadonlySet<string>,
   t: TFunction,
 ): QuestionValidationError[] {
   const linkIndex = buildLinkIndex(questionnaire.questions);
@@ -54,6 +56,9 @@ export function collectStructuredErrors(
         continue;
       }
       const type = question.structured_type;
+      // The slot's component threw — the notice is on screen, not an
+      // input, so there is nothing here to validate.
+      if (renderFailed.has(question.id)) continue;
       const state = resolveStructuredSlotState(
         type,
         questionnaire.subject_type,

src/components/QuestionnaireV2/form/StructuredSlot.tsx

@@ -1,11 +1,13 @@
-import { Suspense, useCallback, useSyncExternalStore } from "react";
+import { Suspense, useCallback, useEffect, useSyncExternalStore } from "react";
 import { useTranslation } from "react-i18next";
 
 import { PluginErrorBoundary } from "@/components/Common/PluginErrorBoundary";
 import { FormSkeleton } from "@/components/Common/SkeletonLoading";
 
 import {
   useClearQuestionErrors,
+  useClearStructuredRenderFailed,
+  useMarkStructuredRenderFailed,
   useQuestionErrors,
   useQuestionResponse,
 } from "@/components/QuestionnaireV2/form/engine/store";
@@ -23,6 +25,24 @@ import type { Question } from "@/types/questionnaire/question";
 
 import { useFormRenderer } from "./FormContext";
 
+/**
+ * Clears the question's render-failed mark when it mounts. Rendered INSIDE
+ * the error boundary, beside the structured component: if the component
+ * throws during the mounting render, the whole subtree — this probe
+ * included — is discarded before effects run, so the clear only ever fires
+ * for a commit whose input actually made it to the screen. That placement
+ * is what makes the mark track reality across an unmount/remount cycle
+ * (enable_when toggling the question): a recovered slot un-exempts itself,
+ * a still-broken one re-marks via the boundary's onError.
+ */
+function ClearRenderFailedOnMount({ questionId }: { questionId: string }) {
+  const clearRenderFailed = useClearStructuredRenderFailed(questionId);
+  useEffect(() => {
+    clearRenderFailed();
+  }, [clearRenderFailed]);
+  return null;
+}
+
 /**
  * Structured questions render through `resolveStructuredType` — core types
  * from STRUCTURED_TYPE_REGISTRY, plugin types from the runtime registry,
@@ -56,6 +76,7 @@ export function StructuredSlot({
   const [response, updateResponse] = useQuestionResponse(question.id);
   const errors = useQuestionErrors(question.id);
   const clearErrors = useClearQuestionErrors(question.id);
+  const markRenderFailed = useMarkStructuredRenderFailed(question.id);
 
   const handleChange = useCallback(
     (values: ResponseValue[], note?: string) =>
@@ -131,13 +152,18 @@ export function StructuredSlot({
     // the same dashed notice the other degradations use.
     <PluginErrorBoundary
       pluginName={definition.type}
+      // Once the notice is showing there is no input to answer, so
+      // submit-time enforcement must stop requiring one — same reasoning
+      // as the subject-mismatch and missing-context skips.
+      onError={markRenderFailed}
       fallback={
         <div className="rounded-lg border border-dashed border-amber-300 bg-amber-50 p-4 text-sm text-amber-800">
           <p className="font-medium">{label}</p>
           <p>{t("structured_question_render_failed")}</p>
         </div>
       }
     >
+      <ClearRenderFailedOnMount questionId={question.id} />
       {/* Plugin components arrive through React.lazy — the boundary keeps
           a still-loading remote from suspending the whole form. */}
       <Suspense fallback={<FormSkeleton rows={2} />}>

src/components/QuestionnaireV2/form/engine/store.ts

@@ -25,6 +25,59 @@ export const questionnaireAtom = atom<QuestionnaireRead | null>(null);
 export const responsesAtom = atom<Record<string, QuestionnaireResponse>>({});
 export const errorsAtom = atom<QuestionValidationError[]>([]);
 
+/**
+ * Question ids whose structured slot THREW during render and are now
+ * showing the error boundary's notice instead of an input.
+ *
+ * Submit-time enforcement reads this alongside the subject-mismatch and
+ * missing-context cases: all three mean "this question has no input on
+ * screen", and requiring an unanswerable question makes the entire form —
+ * every other answer included — permanently unsubmittable. Lives in the
+ * store because the boundary that discovers it and the validators that
+ * must respect it never meet in the component tree.
+ */
+export const structuredRenderFailedAtom = atom<ReadonlySet<string>>(
+  new Set<string>(),
+);
+
+/** Record a structured slot's render failure. Idempotent: re-entering the
+ *  boundary for a question already marked keeps the same Set identity, so
+ *  it cannot loop a subscriber. */
+export function useMarkStructuredRenderFailed(questionId: string) {
+  const markAtom = useMemo(
+    () =>
+      atom(null, (get, set) => {
+        const failed = get(structuredRenderFailedAtom);
+        if (failed.has(questionId)) return;
+        set(structuredRenderFailedAtom, new Set(failed).add(questionId));
+      }),
+    [questionId],
+  );
+  return useAtom(markAtom)[1];
+}
+
+/** Clear a question's render-failed mark — the recovery half of the pair
+ *  above. A slot unmounts and remounts whenever enable_when toggles it (or
+ *  an ancestor group), and the fresh boundary may well render fine; the
+ *  mark must not outlive the notice it described, or a LIVE required input
+ *  would stay exempt from validation for the rest of the session.
+ *  Idempotent the same way: clearing an unmarked question keeps the Set
+ *  identity. */
+export function useClearStructuredRenderFailed(questionId: string) {
+  const clearAtom = useMemo(
+    () =>
+      atom(null, (get, set) => {
+        const failed = get(structuredRenderFailedAtom);
+        if (!failed.has(questionId)) return;
+        const next = new Set(failed);
+        next.delete(questionId);
+        set(structuredRenderFailedAtom, next);
+      }),
+    [questionId],
+  );
+  return useAtom(clearAtom)[1];
+}
+
 /** link_id → question_id for enable_when lookups — pure so non-atom
  *  consumers (form/validation.ts) share the exact same resolution. */
 export function buildLinkIndex(questions: Question[]): Record<string, string> {