Merge pull request #1327 from Open-Source-Legal/claude/add-tests-issue-1276-cTfRS

Raise CT coverage for Corpus Chat & Agent components (fix SYNC_CONTENT loss)

Author: JSv4

CHANGELOG.md

@@ -19,6 +19,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Added
 
+- **Coverage: raise Corpus Chat & Agent Management component tests** (Issue #1276): added 36 new Playwright CT tests across the four lowest-ROI corpus components to drive coverage toward the ≥60% target. Breakdown:
+  - `frontend/tests/CorpusChat.ct.tsx` (+13 tests): `initialQuery` auto-send, tool-call timeline entries (ASYNC_THOUGHT), ASYNC_SOURCES merge, SYNC_CONTENT rendering, ASYNC_RESUME, ask_document sub-tool approval remapping, unknown-type default branch, back-to-list navigation, server-message-with-sources rendering, title-filter debounce, and additional navigation-header coverage. Extended the shared `StubSocket` in `beforeEach` with new query-triggered frame sequences.
+  - `frontend/tests/CreateCorpusActionModal.ct.tsx` (+8 tests): analyzer-path validation, inline-agent validation (empty name / empty instructions), existing-agent-selection validation, successful inline-agent mutation, backend error toast, analyzer edit-mode pre-population, and legacy trigger-casing normalization fallback.
+  - `frontend/tests/CorpusAgentManagement.ct.tsx` (+8 tests): query loading state, query error state, multi-tool badge overflow, inactive-status badge, update-mutation happy path, create-mutation backend-error toast, tool deselection, and edit-modal cancel.
+  - `frontend/tests/CorpusDescriptionEditor.ct.tsx` (+7 tests): save failure (`ok: false`), save network-error path, reapply of snapshot-less version, twice-click collapse, Cancel Version Edit reset, fetch-md URL failure, and version-count pluralization.
+  - **Follow-up review polish**: moved `DEFAULT_DOCUMENT_AGENT_INSTRUCTIONS` from `CreateCorpusActionModal.tsx` into `frontend/src/assets/configurations/constants.ts` so both default-instruction strings (moderator + document agent) live in the single constants module per the project's no-magic-strings rule.
 - **Return-type annotations across core models and import/export pipeline** (Issue #1334, follow-up to #1331): The mypy gate wired in by #1331 recorded a 7208-error baseline frozen across 357 files. This PR pays down the annotation deficit on the core domain models and the bulk import/export tasks without touching runtime behavior or adding validators. Coverage jumped from the pre-issue numbers to:
   - `opencontractserver/corpuses/` 61.5% → 88.4% (target ≥80%)
   - `opencontractserver/annotations/` 48.1% → 93.8% (target ≥80%)
@@ -69,6 +75,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+- **`CorpusChat` dropped `SYNC_CONTENT` messages from the visible chat** (Issue #1276, `frontend/src/components/corpuses/CorpusChat.tsx:468-505`): The `SYNC_CONTENT` WebSocket frame is a standalone, non-streaming assistant reply used for synchronous server responses. `ChatTray` (document chat) appends these directly to its `chat` state; the corpus-level chat only forwarded the content to `handleCompleteMessage`, which stores sources in `ChatSourceAtom` but never pushes a message to the visible list. As a result, any `SYNC_CONTENT` the backend sent over the corpus socket rendered nothing. Fixed by mirroring the `ChatTray` pattern — push a new complete assistant message into `chat` before persisting sources/timeline. The fallback `crypto.randomUUID()` is also now captured in a single local variable so the visible chat entry and the `ChatSourceAtom` record share the same id when the server omits `message_id`. New regression test in `frontend/tests/CorpusChat.ct.tsx` ("SYNC_CONTENT renders a complete message immediately") pins the behavior.
+- **CAML article preview crashed when inserting an extract grid embed** (`frontend/src/utils/camlComponents.ts`, `frontend/src/hooks/useCamlComponentRenderer.tsx`, `frontend/src/components/corpuses/CamlArticleEditor.tsx`, `frontend/src/components/corpuses/caml/CamlDirectiveRenderer.tsx`): the editor wrapped each newly-inserted `[component:TYPE ...]` marker in a `::: prose` fence, but `@os-legal/caml`'s parser has no `case "prose"` in `parseBlock`, so the resulting block carried `body` instead of `content`. `ProseBlock` then crashed inside `splitPullquotes(undefined)`, which unmounted the entire editor modal and made the "ArrowDown then Enter inserts the extract-grid component marker" CT test fail. Switched the fence to a project-specific `::: oc-component` block and routed it through `CamlArticle`'s `customBlocks` slot, where the marker text is handed back to the existing `[component:...]` resolver. The keyboard handler in `CamlArticleEditor` was also tightened to read the active picker index from a `useRef` mirror so back-to-back ArrowDown/Enter keystrokes don't observe a stale closure value of `-1` and bail out before insertion.
 - **PR #1177 follow-up: CAML extract embed polish** (Issue #1227):
   - **`fullDatacellList` payload now bounded server-side**: `ExtractType.full_datacell_list` accepts optional `limit` / `offset` arguments and the resolver clamps `limit` to `MAX_FULL_DATACELL_LIST_LIMIT` (`opencontractserver/constants/extracts.py`, currently `500`) after permission filtering (`config/graphql/extract_types.py`). `GET_EXTRACT_GRID_EMBED` passes `limit: EXTRACT_GRID_EMBED_CELL_LIMIT` (mirrored at `500` in `frontend/src/assets/configurations/constants.ts`) so pathological extracts no longer transmit thousands of cells just to trigger the too-many-rows guard (`frontend/src/graphql/queries.ts`, `frontend/src/components/extracts/ExtractGridEmbed.tsx`). Full server-side pagination is still tracked in #1204.
   - **`resolveComponentMarker` now receives a stable React key from both call sites**: `useCamlComponentRenderer` and `CamlDirectiveRenderer` pass the marker string as the `key` argument so multiple `[component:...]` blocks in a single article reconcile correctly without React's "missing key prop" warnings (`frontend/src/hooks/useCamlComponentRenderer.tsx`, `frontend/src/components/corpuses/caml/CamlDirectiveRenderer.tsx`). Added regression tests in `frontend/src/utils/__tests__/camlComponents.test.ts`.

docs/assets/images/screenshots/auto/corpus--agent-management--agent-row.png

@@ -529,3 +529,18 @@ export const TRIGGER_LABELS: Record<string, string> = {
   new_thread: "On Thread",
   new_message: "On Message",
 } as const;
+
+// Default agent task instructions used when creating a thread-moderation
+// CorpusAction (rendered as the placeholder/initial value in the modal).
+export const DEFAULT_MODERATOR_INSTRUCTIONS = `You are a thread moderator for this corpus. Your role is to:
+1. Monitor discussion threads and messages for policy compliance
+2. Take appropriate moderation actions when needed
+3. Respond helpfully to user questions when appropriate
+
+You have access to thread context, messages, and moderation tools. Use them judiciously.`;
+
+// Default agent task instructions used when creating a document-processing
+// CorpusAction (rendered as the initial value when an add/edit document
+// trigger is selected).
+export const DEFAULT_DOCUMENT_AGENT_INSTRUCTIONS =
+  "You are a document processing agent for this corpus.";

docs/assets/images/screenshots/auto/corpus--chat--conversation-list.png

@@ -373,6 +373,11 @@ export const CamlArticleEditor: React.FC<CamlArticleEditorProps> = ({
   // Index of the keyboard-focused option within the extract picker dropdown.
   // `-1` means no option is focused (initial state when the dropdown opens).
   const [activeExtractIndex, setActiveExtractIndex] = useState<number>(-1);
+  // Mirror of `activeExtractIndex` in a ref so that the keyboard handler can
+  // read the latest value when consecutive keystrokes (e.g. ArrowDown then
+  // Enter) arrive faster than React schedules a re-render. Without this, the
+  // Enter handler would observe a stale closure value of `-1` and bail out.
+  const activeExtractIndexRef = useRef<number>(-1);
   const textareaRef = useRef<HTMLTextAreaElement>(null);
   const extractPickerRef = useRef<HTMLDivElement>(null);
   const extractPickerTriggerRef = useRef<HTMLButtonElement>(null);
@@ -545,9 +550,11 @@ export const CamlArticleEditor: React.FC<CamlArticleEditorProps> = ({
   }, [showExtractPicker]);
 
   // Reset the keyboard-focused option whenever the picker closes so the next
-  // open starts in a clean state.
+  // open starts in a clean state. The ref mirror is reset alongside state so
+  // the next ArrowDown reads the fresh `-1` value synchronously.
   useEffect(() => {
     if (!showExtractPicker) {
+      activeExtractIndexRef.current = -1;
       setActiveExtractIndex(-1);
     }
   }, [showExtractPicker]);
@@ -600,43 +607,57 @@ export const CamlArticleEditor: React.FC<CamlArticleEditorProps> = ({
       if (!showExtractPicker) return;
       const count = corpusExtracts.length;
 
+      // Helper: update both state (drives render) and ref (drives next
+      // synchronous keystroke). The ref read in the Enter case below would
+      // otherwise observe a stale `-1` if Enter arrives in the same tick as
+      // a preceding ArrowDown.
+      const updateActiveIndex = (next: number) => {
+        activeExtractIndexRef.current = next;
+        setActiveExtractIndex(next);
+      };
+
       switch (event.key) {
         case "Escape":
           event.preventDefault();
           setShowExtractPicker(false);
           extractPickerTriggerRef.current?.focus();
           break;
-        case "ArrowDown":
+        case "ArrowDown": {
           if (count === 0) return;
           event.preventDefault();
-          setActiveExtractIndex((prev) => (prev + 1 >= count ? 0 : prev + 1));
+          const prev = activeExtractIndexRef.current;
+          updateActiveIndex(prev + 1 >= count ? 0 : prev + 1);
           break;
-        case "ArrowUp":
+        }
+        case "ArrowUp": {
           if (count === 0) return;
           event.preventDefault();
           // `prev <= 0` covers both `0` (first item → wrap to last) and `-1`
           // (no item focused → jump to last). This is intentional per WAI-ARIA
           // Authoring Practices for listbox keyboard interaction.
-          setActiveExtractIndex((prev) => (prev <= 0 ? count - 1 : prev - 1));
+          const prev = activeExtractIndexRef.current;
+          updateActiveIndex(prev <= 0 ? count - 1 : prev - 1);
           break;
+        }
         case "Home":
           if (count === 0) return;
           event.preventDefault();
-          setActiveExtractIndex(0);
+          updateActiveIndex(0);
           break;
         case "End":
           if (count === 0) return;
           event.preventDefault();
-          setActiveExtractIndex(count - 1);
+          updateActiveIndex(count - 1);
           break;
         case "Enter": {
           if (count === 0) return;
           // Only act when a menu option is focused — otherwise let the
           // default button behaviour on the trigger toggle the picker.
-          if (activeExtractIndex < 0 || activeExtractIndex >= count) return;
+          const current = activeExtractIndexRef.current;
+          if (current < 0 || current >= count) return;
           event.preventDefault();
           event.stopPropagation();
-          const selected = corpusExtracts[activeExtractIndex];
+          const selected = corpusExtracts[current];
           if (selected) {
             // handleInsertComponent already calls setShowExtractPicker(false)
             // internally, so the picker is closed as part of the insertion.
@@ -649,16 +670,14 @@ export const CamlArticleEditor: React.FC<CamlArticleEditorProps> = ({
           break;
       }
     },
-    [
-      showExtractPicker,
-      corpusExtracts,
-      activeExtractIndex,
-      handleInsertComponent,
-    ]
+    [showExtractPicker, corpusExtracts, handleInsertComponent]
   );
 
   // Markdown renderer with generic component marker interception
-  const renderMarkdownPreview = useCamlComponentRenderer(CAML_COMPONENTS);
+  const {
+    renderMarkdown: renderMarkdownPreview,
+    customBlocks: previewCustomBlocks,
+  } = useCamlComponentRenderer(CAML_COMPONENTS);
 
   const handleClose = () => {
     if (hasChanges) {
@@ -744,7 +763,10 @@ export const CamlArticleEditor: React.FC<CamlArticleEditorProps> = ({
                           key={ext.id}
                           $active={index === activeExtractIndex}
                           aria-selected={false}
-                          onMouseEnter={() => setActiveExtractIndex(index)}
+                          onMouseEnter={() => {
+                            activeExtractIndexRef.current = index;
+                            setActiveExtractIndex(index);
+                          }}
                           onClick={() =>
                             handleInsertComponent("extract-grid", {
                               extractId: ext.id,
@@ -778,6 +800,7 @@ export const CamlArticleEditor: React.FC<CamlArticleEditorProps> = ({
                 <CamlArticle
                   document={parsedDocument}
                   renderMarkdown={renderMarkdownPreview}
+                  customBlocks={previewCustomBlocks}
                 />
               </CamlThemeProvider>
             )}

docs/assets/images/screenshots/auto/corpus--chat--empty.png

@@ -466,6 +466,28 @@ export const CorpusChat: React.FC<CorpusChatProps> = ({
             setIsProcessing(false);
             break;
           case "SYNC_CONTENT": {
+            // SYNC_CONTENT is a standalone (non-streaming) assistant reply — unlike the
+            // ASYNC path, it must be appended to `chat` directly or it will never render.
+            // No setIsProcessing(false) is needed: ASYNC_START is the only setter for
+            // isProcessing(true), and SYNC_CONTENT arrives without a preceding ASYNC_START.
+            //
+            // Capture the message id ONCE so the visible chat entry and the
+            // ChatSourceAtom record agree even if the server omits message_id
+            // (otherwise each `crypto.randomUUID()` call produces a different
+            // value, leaving citations unable to find their parent message).
+            const messageId = data?.message_id ?? crypto.randomUUID();
+            setChat((prev) => [
+              ...prev,
+              {
+                messageId,
+                user: "Assistant",
+                content,
+                timestamp: new Date().toLocaleString(),
+                isAssistant: true,
+                isComplete: true,
+              },
+            ]);
+
             const sourcesToPass =
               data?.sources && Array.isArray(data.sources)
                 ? data.sources
@@ -477,7 +499,7 @@ export const CorpusChat: React.FC<CorpusChatProps> = ({
             handleCompleteMessage(
               content,
               sourcesToPass,
-              data?.message_id,
+              messageId,
               undefined,
               timelineToPass
             );
@@ -993,6 +1015,7 @@ export const CorpusChat: React.FC<CorpusChatProps> = ({
         {isConversation && (
           <ChatNavigationHeader>
             <BackButton
+              aria-label="Back to conversation list"
               onClick={(e) => {
                 e.preventDefault();
                 e.stopPropagation();