Skip to content

Commit 4a1a32c

Browse files
committed
Merge remote-tracking branch 'origin/main' into claude/fix-issue-1360-rT6dl
# Conflicts: # CHANGELOG.md
2 parents 2e79b5d + f40e91f commit 4a1a32c

21 files changed

Lines changed: 1824 additions & 71 deletions

CHANGELOG.md

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
77

88
## [Unreleased]
99

10+
### Fixed
11+
12+
- **Merged `frontend` Codecov flag drops to ~33% on every commit where Frontend CI's CT job fails** (`frontend/package.json` `test:coverage:ct`): the script chained `playwright test ... && mkdir -p ... && nyc report ...`, so a failing CT run short-circuited before `nyc report` could turn the per-test JSON files in `.nyc_output` into an `lcov.info`. The downstream `Upload CT Coverage to Codecov` step (`if: success() || failure()`) then errored with "No coverage reports found" and `frontend-component` did not upload for that SHA. Codecov's server-side aggregation of the `frontend` flag was left with only `frontend-unit` (~23%) and `frontend-e2e` (~24%), pulling the merged number down to ~33% even though the previous commit was at ~67% — observed on six consecutive main commits 2026-04-26T01:02..02:58Z (`2d7033f8`..`be5bcfc8`) before recovering on `30298391`. Mirrored the existing `test:e2e:coverage` pattern (`; CT_EXIT=$?; nyc report ... || echo "No coverage data to report"; exit $CT_EXIT`) so `nyc report` runs regardless of test outcome and the lcov ships even on red CT runs. `frontend-component` will still report a slightly lower number when tests fail (failed tests register fewer hits), but it will report — keeping the merged `frontend` flag's denominator stable.
13+
- **`User.__init__` shared-state mutation re-introduced by branch merge** (`opencontractserver/users/models.py:172-180` removed): PR #1374 (commit `50ed6740`) deleted the `User.__init__` override that mutated `Field.validators[0]` on every instantiation, but a subsequent merge (`b68c1cb4 → 6d2cddbf`) resurrected the override along with its mypy-narrowing changes. The current main on commit `6d2cddbf` therefore reproduced the original `#1358` bug: `User(...)` rebound `username_field.validators[0]` and clobbered any third-party validator prepended to the list. Removed the `__init__` override entirely; the class-body declaration `validators=[UserUnicodeUsernameValidator()]` on the `username` field (still present from PR #1374) is the canonical and only declaration. Also dropped the now-unused `Field` import. Regression coverage from PR #1374 (`opencontractserver/tests/test_user_username_validator.py`) was already on main and is what surfaced the regression in CI.
14+
1015
### Security
1116

1217
- **Cross-corpus structural-annotation leak in `CoreAnnotationVectorStore`** (`opencontractserver/llms/vector_stores/core_vector_stores.py:296-326,371-413`): The corpus-wide retrieval path (`corpus_id` set, `document_id=None`) returned every structural annotation in the database regardless of corpus. Two collaborating defects caused the leak:
@@ -19,6 +24,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1924

2025
### Added
2126

27+
- **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:
28+
- `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.
29+
- `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.
30+
- `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.
31+
- `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.
32+
- **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.
2233
- **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:
2334
- `opencontractserver/corpuses/` 61.5% → 88.4% (target ≥80%)
2435
- `opencontractserver/annotations/` 48.1% → 93.8% (target ≥80%)
@@ -76,6 +87,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
7687
- Fixed pre-existing `to_global_id(graphene_model.__class__.__name__, obj.id)` in both `DRFMutation` create/update return paths: `graphene_model` is the `DjangoObjectType` class, so `.__class__` referenced graphene's metaclass (`SubclassWithMeta_Meta`) and the resulting global id used the wrong type name. Switched to `graphene_model.__name__` to match the GraphQL type name (e.g. `"CorpusType"`) used everywhere else in `config/graphql/`. Surfaced by the new explicit `ClassVar[Optional[type[...]]]` annotations — both call sites stayed dormant because the returned `obj_id` is rarely consumed by the frontend (which re-queries by global id from its own cache).
7788
- Graduated `config.graphql.base` out of the mypy baseline: removed the `[mypy-config.graphql.base]` section from `mypy.ini` and pruned the ten corresponding lines from `docs/typing/mypy_baseline.txt`. `mypy --config-file mypy.ini` is now clean on this module.
7889
- Regression tests in `opencontractserver/tests/test_security_hardening.py::TestIOSettingsRequiredFieldsGuard`: the helper raises `NotImplementedError` for each of `model`/`serializer`/`graphene_model` when missing or `None`, returns the configured value when present, and the base classes expose the `None` defaults the guard relies on.
90+
- **`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.
91+
- **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.
7992
- **PR #1177 follow-up: CAML extract embed polish** (Issue #1227):
8093
- **`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.
8194
- **`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`.
55.3 KB
Loading
2.36 KB
Loading
3.59 KB
Loading
-114 Bytes
Loading
48.1 KB
Loading
53.5 KB
Loading

frontend/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@
100100
"test:e2e": "playwright test",
101101
"test:e2e:coverage": "COVERAGE=true playwright test; E2E_EXIT=$?; mkdir -p coverage/e2e/.nyc_output && nyc report --all --reporter=lcov --reporter=text --report-dir=coverage/e2e --temp-dir=coverage/e2e/.nyc_output || echo 'No coverage data to report'; exit $E2E_EXIT",
102102
"test:coverage:unit": "vitest run --coverage --watch=false",
103-
"test:coverage:ct": "COVERAGE=true playwright test -c playwright-ct.config.ts --reporter=list && mkdir -p coverage/ct/.nyc_output && nyc report --all --reporter=lcov --reporter=text --report-dir=coverage/ct --temp-dir=coverage/ct/.nyc_output",
103+
"test:coverage:ct": "COVERAGE=true playwright test -c playwright-ct.config.ts --reporter=list; CT_EXIT=$?; mkdir -p coverage/ct/.nyc_output && nyc report --all --reporter=lcov --reporter=text --report-dir=coverage/ct --temp-dir=coverage/ct/.nyc_output || echo 'No coverage data to report'; exit $CT_EXIT",
104104
"lint": "prettier . --check --ignore-unknown",
105105
"fix-styles": "prettier . --check --write --ignore-unknown",
106106
"prepare": "cd .. && husky install frontend/.husky",

frontend/src/assets/configurations/constants.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -529,3 +529,18 @@ export const TRIGGER_LABELS: Record<string, string> = {
529529
new_thread: "On Thread",
530530
new_message: "On Message",
531531
} as const;
532+
533+
// Default agent task instructions used when creating a thread-moderation
534+
// CorpusAction (rendered as the placeholder/initial value in the modal).
535+
export const DEFAULT_MODERATOR_INSTRUCTIONS = `You are a thread moderator for this corpus. Your role is to:
536+
1. Monitor discussion threads and messages for policy compliance
537+
2. Take appropriate moderation actions when needed
538+
3. Respond helpfully to user questions when appropriate
539+
540+
You have access to thread context, messages, and moderation tools. Use them judiciously.`;
541+
542+
// Default agent task instructions used when creating a document-processing
543+
// CorpusAction (rendered as the initial value when an add/edit document
544+
// trigger is selected).
545+
export const DEFAULT_DOCUMENT_AGENT_INSTRUCTIONS =
546+
"You are a document processing agent for this corpus.";

frontend/src/components/corpuses/CamlArticleEditor.tsx

Lines changed: 40 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -373,6 +373,11 @@ export const CamlArticleEditor: React.FC<CamlArticleEditorProps> = ({
373373
// Index of the keyboard-focused option within the extract picker dropdown.
374374
// `-1` means no option is focused (initial state when the dropdown opens).
375375
const [activeExtractIndex, setActiveExtractIndex] = useState<number>(-1);
376+
// Mirror of `activeExtractIndex` in a ref so that the keyboard handler can
377+
// read the latest value when consecutive keystrokes (e.g. ArrowDown then
378+
// Enter) arrive faster than React schedules a re-render. Without this, the
379+
// Enter handler would observe a stale closure value of `-1` and bail out.
380+
const activeExtractIndexRef = useRef<number>(-1);
376381
const textareaRef = useRef<HTMLTextAreaElement>(null);
377382
const extractPickerRef = useRef<HTMLDivElement>(null);
378383
const extractPickerTriggerRef = useRef<HTMLButtonElement>(null);
@@ -545,9 +550,11 @@ export const CamlArticleEditor: React.FC<CamlArticleEditorProps> = ({
545550
}, [showExtractPicker]);
546551

547552
// Reset the keyboard-focused option whenever the picker closes so the next
548-
// open starts in a clean state.
553+
// open starts in a clean state. The ref mirror is reset alongside state so
554+
// the next ArrowDown reads the fresh `-1` value synchronously.
549555
useEffect(() => {
550556
if (!showExtractPicker) {
557+
activeExtractIndexRef.current = -1;
551558
setActiveExtractIndex(-1);
552559
}
553560
}, [showExtractPicker]);
@@ -600,43 +607,57 @@ export const CamlArticleEditor: React.FC<CamlArticleEditorProps> = ({
600607
if (!showExtractPicker) return;
601608
const count = corpusExtracts.length;
602609

610+
// Helper: update both state (drives render) and ref (drives next
611+
// synchronous keystroke). The ref read in the Enter case below would
612+
// otherwise observe a stale `-1` if Enter arrives in the same tick as
613+
// a preceding ArrowDown.
614+
const updateActiveIndex = (next: number) => {
615+
activeExtractIndexRef.current = next;
616+
setActiveExtractIndex(next);
617+
};
618+
603619
switch (event.key) {
604620
case "Escape":
605621
event.preventDefault();
606622
setShowExtractPicker(false);
607623
extractPickerTriggerRef.current?.focus();
608624
break;
609-
case "ArrowDown":
625+
case "ArrowDown": {
610626
if (count === 0) return;
611627
event.preventDefault();
612-
setActiveExtractIndex((prev) => (prev + 1 >= count ? 0 : prev + 1));
628+
const prev = activeExtractIndexRef.current;
629+
updateActiveIndex(prev + 1 >= count ? 0 : prev + 1);
613630
break;
614-
case "ArrowUp":
631+
}
632+
case "ArrowUp": {
615633
if (count === 0) return;
616634
event.preventDefault();
617635
// `prev <= 0` covers both `0` (first item → wrap to last) and `-1`
618636
// (no item focused → jump to last). This is intentional per WAI-ARIA
619637
// Authoring Practices for listbox keyboard interaction.
620-
setActiveExtractIndex((prev) => (prev <= 0 ? count - 1 : prev - 1));
638+
const prev = activeExtractIndexRef.current;
639+
updateActiveIndex(prev <= 0 ? count - 1 : prev - 1);
621640
break;
641+
}
622642
case "Home":
623643
if (count === 0) return;
624644
event.preventDefault();
625-
setActiveExtractIndex(0);
645+
updateActiveIndex(0);
626646
break;
627647
case "End":
628648
if (count === 0) return;
629649
event.preventDefault();
630-
setActiveExtractIndex(count - 1);
650+
updateActiveIndex(count - 1);
631651
break;
632652
case "Enter": {
633653
if (count === 0) return;
634654
// Only act when a menu option is focused — otherwise let the
635655
// default button behaviour on the trigger toggle the picker.
636-
if (activeExtractIndex < 0 || activeExtractIndex >= count) return;
656+
const current = activeExtractIndexRef.current;
657+
if (current < 0 || current >= count) return;
637658
event.preventDefault();
638659
event.stopPropagation();
639-
const selected = corpusExtracts[activeExtractIndex];
660+
const selected = corpusExtracts[current];
640661
if (selected) {
641662
// handleInsertComponent already calls setShowExtractPicker(false)
642663
// internally, so the picker is closed as part of the insertion.
@@ -649,16 +670,14 @@ export const CamlArticleEditor: React.FC<CamlArticleEditorProps> = ({
649670
break;
650671
}
651672
},
652-
[
653-
showExtractPicker,
654-
corpusExtracts,
655-
activeExtractIndex,
656-
handleInsertComponent,
657-
]
673+
[showExtractPicker, corpusExtracts, handleInsertComponent]
658674
);
659675

660676
// Markdown renderer with generic component marker interception
661-
const renderMarkdownPreview = useCamlComponentRenderer(CAML_COMPONENTS);
677+
const {
678+
renderMarkdown: renderMarkdownPreview,
679+
customBlocks: previewCustomBlocks,
680+
} = useCamlComponentRenderer(CAML_COMPONENTS);
662681

663682
const handleClose = () => {
664683
if (hasChanges) {
@@ -744,7 +763,10 @@ export const CamlArticleEditor: React.FC<CamlArticleEditorProps> = ({
744763
key={ext.id}
745764
$active={index === activeExtractIndex}
746765
aria-selected={false}
747-
onMouseEnter={() => setActiveExtractIndex(index)}
766+
onMouseEnter={() => {
767+
activeExtractIndexRef.current = index;
768+
setActiveExtractIndex(index);
769+
}}
748770
onClick={() =>
749771
handleInsertComponent("extract-grid", {
750772
extractId: ext.id,
@@ -778,6 +800,7 @@ export const CamlArticleEditor: React.FC<CamlArticleEditorProps> = ({
778800
<CamlArticle
779801
document={parsedDocument}
780802
renderMarkdown={renderMarkdownPreview}
803+
customBlocks={previewCustomBlocks}
781804
/>
782805
</CamlThemeProvider>
783806
)}

0 commit comments

Comments
 (0)