Merge pull request #1462 from Open-Source-Legal/claude/cross-content-search-gi0zc

Add cross-content Discover search with notes integration

Author: JSv4

CHANGELOG.md

@@ -9,6 +9,49 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Added
 
+- **Cross-content Discover search** at `/discover/search`. The Discover hero
+  search box now lands on a unified results page that fans out across
+  discussions, annotations, collections (corpuses), and notes in parallel,
+  rather than redirecting to discussion-only search.
+  - Backend: new `searchNotesForMention` resolver in
+    `config/graphql/search_queries.py` (visibility via
+    `Note.objects.visible_to_user`, optional `corpusId` / `documentId`
+    scoping, eager-loads `document`, `corpus`, and creators).
+    `NoteType` is exported through the same `DjangoConnectionField` shape
+    used by the other mention searches and goes through `NoteType.get_queryset`
+    for a second visibility pass.
+  - Frontend view: `frontend/src/views/DiscoverSearchResults.tsx` reuses
+    `ThreadListItem` for discussions and a single shared `ResultRow` for
+    annotations / collections / notes. Tabbed UI: All (preview of 5 each),
+    Discussions, Annotations, Collections, Notes.
+  - Routing: `?q=` and `?type=` are URL-driven (replace, no history spam).
+    Result rows deep-link via `getCorpusUrl` / `getDocumentUrl`; annotation
+    rows preserve `?ann=`, note rows pass `?note=` to the document URL.
+  - New `?note=<id>` deep-link param wired through `selectedNoteId`
+    (`frontend/src/graphql/cache.ts`), `QueryParams.noteId`
+    (`frontend/src/utils/navigationUtils.ts`), and the central manager
+    Phase 2/4 sync (`frontend/src/routing/CentralRouteManager.tsx`).
+    `DocumentKnowledgeBase` now consumes the var: when the loaded
+    document contains a note matching the URL id, the existing note
+    detail modal opens once and the param is consumed (one-shot
+    deep-link, mirrored back to the URL via Phase 4).
+  - Hero wiring: `NewHeroSection.handleSearchSubmit` now navigates to
+    `/discover/search?q=...`. The legacy `/discussions?search=...` route is
+    preserved for callers that want a discussion-only listing.
+  - Tests: `MentionSearchTestCase` in
+    `opencontractserver/tests/test_mentions.py` covers visibility
+    filtering, content-substring matching, corpus / document scoping,
+    anonymous-user filtering, wrong-type global-id rejection, the
+    `-modified` ordering contract, and both
+    `content_preview` paths (DB-annotated `Left('content', 400)` for
+    search results, Python fallback for per-id note fetch).
+  - Component test: `frontend/tests/DiscoverSearchResults.ct.tsx`
+    covers the empty-prompt state, the four-section header render
+    after typing, and a populated-results render exercising every
+    section's row primitives. Captures both
+    `discover--search-results--empty-prompt.png` and
+    `discover--search-results--with-results.png` for docs.
+
 - **Loud guardrail against the `system_prompt=` foot-gun in pydantic-ai** (Issue #1451): `pydantic_ai.Agent` accepts both `system_prompt=` and `instructions=`, but the `system_prompt` value is *only* materialised into the model request when `message_history` is `None`. OpenContracts' `chat()` flow always persists the user's HUMAN message before calling `Agent.run()`, so `message_history` is never empty in practice and any `system_prompt=` argument is silently dropped — the LLM runs without any system instruction. CLAUDE.md pitfall #14 documented the workaround (use `instructions=`), but a future pydantic-ai bump that renames or re-precedences these parameters could re-introduce the regression silently.
   - **Single construction path** (`opencontractserver/llms/agents/pydantic_ai_factory.py`): new `make_pydantic_ai_agent(...)` factory is now the only place in the codebase that instantiates `pydantic_ai.Agent`. The factory uses a sentinel-based check (not `is not None`) to refuse `system_prompt=` outright — even `system_prompt=None` raises `TypeError` so the lesson cannot be re-learned by accident. The error message references issue #1451 and CLAUDE.md pitfall #14.
   - **All call sites refactored** (in `opencontractserver/llms/agents/pydantic_ai_agents.py`: `_run_structured_extraction`, the document-agent factory, and the corpus-agent factory; in `opencontractserver/tasks/memory_tasks.py`: `summarise_agent` and `curation_agent`). Five direct `PydanticAIAgent(...)` constructions in production code now route through the factory.
@@ -31,6 +74,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+- **Discover search section header miscounted threads when soft-deleted ones were present** (`frontend/src/views/DiscoverSearchResults.tsx`): the discussions section displayed `data.conversations.totalCount` as the count, but rendered rows are filtered client-side via `!n?.deletedAt`. With even one tombstoned thread in the results, the header read e.g. "5 threads" while only 4 rows appeared. Switched the count to `threads.length` (post-filter) so the badge always matches what the user sees.
+
+- **`?note=<id>` deep-link could pin in the URL forever for documents with no notes** (`frontend/src/components/knowledge_base/document/DocumentKnowledgeBase.tsx`): the auto-open effect early-returned on `notes.length === 0`, which is *also* true while the document query is loading. The intent comment said "Clear regardless of match" but the early return skipped the clear call entirely. If the underlying document genuinely had no notes (or the loaded notes simply did not contain the deep-linked id) the reactive var stayed set, so `CentralRouteManager` Phase 4 kept writing `?note=<id>` back into the URL on every render — the deep-link became sticky with no user-visible escape hatch. Effect now gates only on `combinedData?.document` being loaded; once the query has resolved, the effect runs `find` (possibly empty) and always clears the var.
+
+- **`NewHeroSection` had a dead `isAuthenticated` prop** (`frontend/src/components/landing/NewHeroSection.tsx`, `frontend/src/views/DiscoveryLanding.tsx`): the prop was declared on the interface and threaded through `DiscoveryLanding` but never read inside the component. Removed from the interface and the call site.
+
+- **Discover search rows silently no-op'd on `"#"` URLs from missing slugs** (`frontend/src/views/DiscoverSearchResults.tsx`): `getDocumentUrl` / `getCorpusUrl` return `"#"` when slugs are missing on the underlying entities. Each section's `onClick` did `if (url !== "#") navigate(url)`, so the row remained visually clickable, hover styles fired, and the click was swallowed with no feedback. `ResultRow` now accepts `disabled` / `disabledReason`; sections compute `unrouteable = url === "#"` once and forward both. Disabled rows render with `opacity 0.55`, `cursor: not-allowed`, native `disabled` (and `aria-disabled`), and a tooltip explaining the missing data — replacing the silent failure with a discoverable one. `disabled:not(:disabled)` scoping on the hover style stops the disabled rows from animating on hover.
+
 - **Annotation deep-links from the corpus-home Table of Contents silently no-op'd** (`frontend/src/components/corpuses/DocumentAnnotationIndex.tsx`, `frontend/src/components/knowledge_base/document/document_kb/RightPanelContent.tsx`): clicking a structural section in the corpus-home document index (e.g. "Subchapter I. Formation, p. 2") was supposed to open the document with the annotation pre-selected and scrolled into view. Instead it appeared to do nothing. Root cause: `DocumentAnnotationIndex` overloaded a single `embedded` prop with two semantics — visual layout ("render without an outer container") *and* click routing ("we are already on the document page, just rewrite `?ann=`"). The corpus-home call site (`DocumentTableOfContents.tsx:919`) needed the visual flavor but absolutely was *not* on a document page, so `handleSectionClick` took the wrong branch and wrote `?ann=<id>` onto the corpus URL — no navigation, no scroll. Fix splits the prop: `embedded` is now purely visual, and a new explicit `onDocumentPage` prop controls click routing. The single call site that's actually on a document page (`RightPanelContent.tsx`) opts in. Regression test in `frontend/src/components/corpuses/__tests__/DocumentAnnotationIndex.test.tsx` pins the new contract: a click from a corpus URL must produce a string-form `navigate("/d/.../doc?ann=<id>")` (full deep link), while a click from a document URL with `onDocumentPage` produces `navigate({ search: "...ann=<id>..." }, { replace: true })`.
 
 - **Zip importer reported `success: True` even when sidecars failed (silent annotation loss)** (`opencontractserver/tasks/import_tasks.py:421`, `:1411`): `_read_sidecar` raises `ValueError` when a sidecar exceeds `ZIP_MAX_SIDECAR_SIZE_BYTES`; malformed JSON, schema failures, and missing labels for sidecar-declared annotations all bump `annotation_sidecars_errored` and append to `errors`. The success determinations only checked `files_errored` (`import_zip_with_folder_structure`) or the user-cap message (`process_documents_zip`), so callers observed `success: True, completed: True` while annotations were silently dropped — exactly the silent data-loss path called out in PR #1489 review feedback. `import_zip_with_folder_structure` now requires `annotation_sidecars_errored == 0` in addition to the existing `files_errored == 0` and user-cap check; `process_documents_zip` now requires `error_files == 0` in addition to the user-cap check. `relationship_errors` is intentionally not folded in — those are surfaced separately via `relationships_skipped` + `relationship_errors` and the documents themselves are imported correctly. Two tests in `test_sidecar_import.py` (`test_skip_pipeline_without_labels_json`) and a new regression test (`test_sidecar_error_drops_overall_success_flag`) lock down the new contract.

config/graphql/annotation_types.py

@@ -340,6 +340,20 @@ class NoteType(AnnotatePermissionsForReadMixin, DjangoObjectType):
     )
     current_version = graphene.Int(description="Current version number of the note")
 
+    content_preview = graphene.String(
+        description=(
+            "First 400 characters of the note body for list/search previews. "
+            "Resolvers may annotate the queryset with `content_preview` to "
+            "avoid shipping the full body over the wire."
+        )
+    )
+
+    def resolve_content_preview(self, info) -> str:
+        annotated = getattr(self, "content_preview", None)
+        if annotated is not None:
+            return annotated
+        return (self.content or "")[:400]
+
     def resolve_revisions(self, info) -> Any:
         """Returns all revisions for this note, ordered by version."""
         return self.revisions.all()

config/graphql/search_queries.py

@@ -8,6 +8,7 @@
 import graphene
 from django.contrib.postgres.search import SearchQuery
 from django.db.models import Q
+from django.db.models.functions import Left
 from graphene_django.fields import DjangoConnectionField
 from graphql_jwt.decorators import login_required
 from graphql_relay import from_global_id
@@ -17,11 +18,12 @@
     AnnotationType,
     CorpusType,
     DocumentType,
+    NoteType,
     SemanticSearchResultType,
     UserType,
 )
 from config.graphql.ratelimits import get_user_tier_rate, graphql_ratelimit_dynamic
-from opencontractserver.annotations.models import Annotation
+from opencontractserver.annotations.models import Annotation, Note
 from opencontractserver.constants.annotations import SEMANTIC_SEARCH_MAX_RESULTS
 from opencontractserver.constants.search import FTS_CONFIG
 from opencontractserver.corpuses.models import Corpus
@@ -75,6 +77,19 @@ class SearchQueryMixin:
         ),
     )
 
+    search_notes_for_mention = DjangoConnectionField(
+        NoteType,
+        text_search=graphene.String(
+            description="Search query to find notes by title or content"
+        ),
+        corpus_id=graphene.ID(
+            description="Optional corpus ID to scope search to notes in specific corpus"
+        ),
+        document_id=graphene.ID(
+            description="Optional document ID to scope search to notes on a specific document"
+        ),
+    )
+
     @graphql_ratelimit_dynamic(get_rate=get_user_tier_rate("READ_LIGHT"))
     def resolve_search_corpuses_for_mention(
         self, info, text_search=None, **kwargs
@@ -418,6 +433,68 @@ def resolve_search_agents_for_mention(
         # Order: Global first, then corpus-specific, then alphabetically by name
         return qs.select_related("creator", "corpus").order_by("scope", "name")
 
+    @graphql_ratelimit_dynamic(get_rate=get_user_tier_rate("READ_LIGHT"))
+    def resolve_search_notes_for_mention(
+        self, info, text_search=None, corpus_id=None, document_id=None, **kwargs
+    ) -> Any:
+        """
+        Search notes by title or content.
+
+        SECURITY: Notes inherit visibility from document + corpus via
+        `Note.objects.visible_to_user()`. Anonymous users only see notes whose
+        document, corpus (if any), and the note itself are public.
+        """
+        user = info.context.user
+
+        qs = Note.objects.visible_to_user(user)
+
+        # Reject malformed or wrong-type global IDs by returning an empty
+        # queryset rather than silently filtering on a non-existent FK.
+        if corpus_id:
+            try:
+                type_name, corpus_pk = from_global_id(corpus_id)
+            except (ValueError, UnicodeDecodeError):
+                return Note.objects.none()
+            if type_name != "CorpusType":
+                return Note.objects.none()
+            qs = qs.filter(corpus_id=int(corpus_pk))
+
+        if document_id:
+            try:
+                type_name, document_pk = from_global_id(document_id)
+            except (ValueError, UnicodeDecodeError):
+                return Note.objects.none()
+            if type_name != "DocumentType":
+                return Note.objects.none()
+            qs = qs.filter(document_id=int(document_pk))
+
+        if text_search:
+            # TODO(perf): Note has no `search_vector` column today (unlike
+            # Annotation), so `icontains` is the only available substring
+            # matcher. This is `LIKE '%…%'` and cannot use a B-tree or GIN
+            # index — it degrades to a sequential scan as note volume grows
+            # and returns lower-quality matches than FTS (no stemming/rank).
+            # The fix is to add a `SearchVectorField` + GIN index to `Note`,
+            # backfill it, and switch this filter to `SearchQuery` /
+            # `SearchVector` with `FTS_CONFIG` (mirroring
+            # `resolve_search_annotations_for_mention`). Acceptable for the
+            # small note corpora this was tested against.
+            qs = qs.filter(
+                Q(title__icontains=text_search) | Q(content__icontains=text_search)
+            )
+
+        # Eager-load the relations the result row needs for deep-linking, and
+        # annotate a DB-truncated preview so the wire payload doesn't ship the
+        # full markdown body for every result.
+        qs = qs.select_related(
+            "document", "document__creator", "corpus", "creator"
+        ).annotate(content_preview=Left("content", 400))
+
+        # NoteType.get_queryset re-applies `visible_to_user` as a defensive
+        # second pass, so callers cannot widen visibility by bypassing this
+        # resolver.
+        return qs.order_by("-modified")
+
     # SEMANTIC SEARCH QUERIES #############################################
     semantic_search = graphene.List(
         SemanticSearchResultType,

docs/assets/images/screenshots/auto/discover--search-results--empty-prompt.png

@@ -89,6 +89,7 @@ import { LeaderboardRoute } from "./components/routes/LeaderboardRoute";
 import { GlobalDiscussionsRoute } from "./components/routes/GlobalDiscussionsRoute";
 import { ThreadSearchRoute } from "./views/ThreadSearchRoute";
 import { DiscoveryLanding } from "./views/DiscoveryLanding";
+import { DiscoverSearchResults } from "./views/DiscoverSearchResults";
 import { CentralRouteManager } from "./routing/CentralRouteManager";
 import { CRUDModal } from "./components/widgets/CRUD/CRUDModal";
 import { updateAnnotationDisplayParams } from "./utils/navigationUtils";
@@ -481,6 +482,12 @@ export const App = () => {
                   <Route path="/corpuses" element={<Corpuses />} />
                   <Route path="/documents" element={<Documents />} />
 
+                  {/* Cross-content Discover search */}
+                  <Route
+                    path="/discover/search"
+                    element={<DiscoverSearchResults />}
+                  />
+
                   {/* Global Discussions Route (Issue #623) */}
                   <Route
                     path="/discussions"

docs/assets/images/screenshots/auto/discover--search-results--with-results.png

@@ -209,6 +209,14 @@ export const OC_SECTION_LABEL = "OC_SECTION";
 // Document search/picker limits
 export const DOCUMENT_PICKER_SEARCH_LIMIT = 20;
 
+// Discover cross-content search (DiscoverSearchResults view)
+/** Number of results shown per section on the "All" tab (preview mode). */
+export const DISCOVER_SEARCH_ALL_TAB_PREVIEW = 5;
+/** Number of results shown when an entity tab is selected. */
+export const DISCOVER_SEARCH_ENTITY_TAB_LIMIT = 25;
+/** Debounce (ms) before firing cross-content search queries. */
+export const DISCOVER_SEARCH_DEBOUNCE_MS = 250;
+
 // Mutation batching
 export const MUTATION_BATCH_SIZE = 10;
 

frontend/src/App.tsx

@@ -102,6 +102,8 @@ export const OS_LEGAL_COLORS = {
   // Folder-specific colors - amber/golden theme
   /** Folder icon color - amber/golden for visual distinction from documents. */
   folderIcon: "#D97706",
+  /** Darker amber companion to folderIcon (used as gradient end-stop). */
+  folderIconDark: "#b45309",
   /** Folder background gradient - warm amber tones. */
   folderIconBg: "linear-gradient(135deg, #FEF3C7 0%, #FDE68A 100%)",
 

frontend/src/assets/configurations/constants.ts

@@ -148,6 +148,7 @@ import { getDocument, GlobalWorkerOptions } from "pdfjs-dist";
 import workerSrc from "pdfjs-dist/build/pdf.worker.mjs?url";
 import {
   selectedAnnotationIds,
+  selectedNoteId,
   selectedThreadId,
   showStructuralAnnotations,
 } from "../../../graphql/cache";
@@ -1622,6 +1623,22 @@ const DocumentKnowledgeBase: React.FC<DocumentKnowledgeBaseProps> = ({
     }
   }, [threadId, combinedData?.document]);
 
+  // Auto-open the note detail modal when ?note= deep-link is present.
+  const deepLinkedNoteId = useReactiveVar(selectedNoteId);
+  useEffect(() => {
+    // Wait until the document query has resolved before deciding the
+    // ?note=<id> param is unresolvable — `notes` is empty during the
+    // loading window too, and clearing then would race the load.
+    if (!deepLinkedNoteId || !combinedData?.document) return;
+    const target = notes.find((n) => n.id === deepLinkedNoteId);
+    if (target) setSelectedNote(target);
+    // Clear regardless of match: once the document is loaded, a missing
+    // target means the note is inaccessible, deleted, or the ID is stale —
+    // leaving the var set would pin ?note=<id> in the URL forever via
+    // CentralRouteManager.
+    selectedNoteId(null);
+  }, [deepLinkedNoteId, combinedData?.document, notes]);
+
   // The main viewer content:
   let viewerContent: JSX.Element = <></>;
   if (isPdfFileType(metadata.fileType)) {

frontend/src/assets/configurations/osLegalStyles.ts

@@ -11,7 +11,6 @@ import {
 } from "../../graphql/landing-queries";
 
 interface NewHeroSectionProps {
-  isAuthenticated?: boolean;
   selectedCategory: string | null;
   onCategoryChange: (categoryId: string | null) => void;
 }
@@ -65,7 +64,6 @@ const FilterContainer = styled.div`
 `;
 
 export const NewHeroSection: React.FC<NewHeroSectionProps> = ({
-  isAuthenticated,
   selectedCategory,
   onCategoryChange,
 }) => {
@@ -86,7 +84,7 @@ export const NewHeroSection: React.FC<NewHeroSectionProps> = ({
   const handleSearchSubmit = useCallback(
     (value: string) => {
       if (value.trim()) {
-        navigate(`/discussions?search=${encodeURIComponent(value.trim())}`);
+        navigate(`/discover/search?q=${encodeURIComponent(value.trim())}`);
       }
     },
     [navigate]