RFC: search refactoring principles and direction

search-refactoring-principles-rfc.md

@@ -0,0 +1,215 @@
+# Refactoring ArchivesSpace search: principles and direction
+
+*A request for feedback from core committers and operators of production-scale ArchivesSpace installations.*
+
+A whole cluster of search bugs share one root cause: ArchivesSpace dumps **all fields from a record into a single catchall solr field** and searches that, with the relevance rules hard-coded in Ruby. We propose to fix those bugs at the source by **replacing the one-big-field approach with many named, allow-listed Solr fields**, and by **moving relevance tuning out of the Ruby code and into Solr's config files**.
+
+This is a big, structural change to how search works under the hood, so we want to get the direction right before committing to it. This document sets out the principles behind the change, with reference code to make each concrete. It is not the implementation plan (that exists separately).
+
+We want feedback on the **direction** from people who run ArchivesSpace at scale, maintain a plugin that touches Solr, or have opinions about discovery relevance. The questions we most want answered are collected at the end.
+
+Jira epic: [ANW-2229](https://archivesspace.atlassian.net/browse/ANW-2229) (roadmap idea ASRM-27)
+
+---
+
+## 1. The problem we are trying to fix
+
+A cluster of long-standing search defects share one root cause. The symptoms look unrelated to a user:
+
+- A researcher types `45`, gets a resource, opens it, and finds `45` nowhere in the title, notes, dates, or agents. The real match was a subject URI (`/subjects/45`), a top-container barcode, or an internal event ID. (ANW-2657, ANW-201, ANW-1672)
+- A multi-part identifier `MS-2024-001` does not match `MS2024001` or `MS 2024 001`. "Identifier contains" misses obvious substrings. (ANW-290, ANW-1556, ANW-2071, ANW-2075)
+- `[2024]` is searchable only as `2024`; an ampersand breaks a facet-filter URL; multi-term queries return weak partial matches; non-Roman scripts do not fold. (ANW-308, ANW-1686, ANW-859, ANW-1178)
+- A note match is not highlighted in the result-list summary. (ANW-2315)
+- The PUI Creator filter returns records whose only matching creator is unpublished. The Level facet count disagrees with the level shown on the record. (ANW-262, ANW-1580)
+- The Agent linker ranks records where "Smith" appears in a biography above records where "Smith" *is* the agent. (ANW-2656)
+
+The common thread is **architectural, not a set of independent bugs**. Two structural choices make all of these hard to fix:
+
+1. **Catchall fields.** A type-blind indexer (`IndexerCommon.extract_string_values`) tree-walks a record's JSON and concatenates every field into one searchable solr field (`fullrecord` / `fullrecord_published`). A second one does the same for notes. URIs, internal IDs, barcodes, agent contacts etc all land in the same searchable bag as genuine content.
+
+2. **Relevance configuration assembled in Ruby.** The solr query (`qf` / `pf`) string is built in `backend/app/model/solr.rb` and appended to every request. There is no way to weight fields per search context (a name lookup vs. a title search vs. an identifier search).
+
+The proposal is to change these two structural choices so the issues become individually addressable.
+
+---
+
+## 2. The guiding principles
+
+### Principle 1: No catchall fields. Search an explicit allow-list of named fields.
+
+Today the default keyword path searches `fullrecord`, which is built like this (pseudocode version of `indexer_common.rb:140-207`):
+
+```ruby
+# Type-blind: every string anywhere in the record JSON ends up searchable.
+def extract_string_values(record)
+  found_strings = ""
+  # ...recursively walk every value in the nested JSON...
+  # subject URIs, internal IDs, barcodes, agent contacts, staff notes - all of it
+  found_strings
+end
+
+doc['fullrecord'] = build_fullrecord(record)   # one giant searchable blob
+```
+
+The refactoring proposal: **every searchable field is a real Solr field with an explicit name, and relevance configuration enumerates those names directly. A field that is not enumerated in any relevance group is not searchable by default.**
+
+This turns an unbounded *deny-list* problem ("what do we need to keep out of `fullrecord`?", which is unanswerable because the walker is type-blind) into a finite, reviewable *allow-list* ("what do we want searchable?"). Defects become attributable to a single source field. Per-field decisions (analyzer, highlighting, publish-scope, facet routing) become possible because there is a field to attach the decision to.
+
+Note this goes one step further than ArcLight, our closest reference (see §4). ArcLight keeps a `text` catchall fed by `<copyField>`. We propose no catchall at all: the relevance group enumerates the source fields. Adding a field to default search means editing one config entry. URIs and internal IDs are not included in the list.
+
+### Principle 2: Relevance configuration lives in Solr config, not in Ruby.
+
+Today (`solr.rb:345-350`), every request gets a solr query (`qf` string) built in Ruby:
+
+```ruby
+# One global qf, differenciating only on publish-protection, appended to every request.
+qf = "identifier_ws^3 title_ws^2 finding_aid_filing_title^2 fullrecord"
+```
+
+The proposal moves relevance into named paramsets in `solrconfig.xml`, invoked per request by name:
+
+```xml
+<initParams path="/select">
+  <lst name="defaults">
+    <str name="qf_default">title_tesim^5 scopecontent_tesim bioghist_tesim creators_text subjects_text ...</str>
+    <str name="qf_name">primary_name_tesim^10 creators_text^3 ...</str>
+    <str name="qf_identifier">identifier_match^10 ead_id ref_id component_id unitid_ssm ...</str>
+    <str name="mm">4&lt;90%</str>
+  </lst>
+</initParams>
+```
+
+```ruby
+# The backend's per-request work shrinks to choosing a paramset by name.
+# Agent linker -> qf_name; Subject linker -> qf_subject; identifier context -> qf_identifier.
+params[:useParams] = "qf_name"
+```
+
+Benefits: relevance is **tunable through solr configuration** (edit `solrconfig.xml`, reload), **auditable in one place** instead of spread across request-building code, and **composable** by reusing / combining solr paramSets (`useParams=qf_default,locale_boost`). It also directly fixes the typeahead-relevancy issue (ANW-2656): the search on the Agent linker can use a specific paramset to prioritise name fields.
+
+### Principle 3: Per-field analyzer choices, not one analyzer everywhere.
+
+Today every text field uses one `text_general` analyzer. That single choice causes the bracket, ampersand, partial-match, and non-Roman-script defects at once. The proposal assigns analyzers per field family:
+
+- **`text_icu`** (ICU tokenizer + ICU folding) backs the text-search fields. ArchivesSpace supports many languages and scripts for archival description, so this covers Cyrillic, Arabic, Hebrew, Greek, and CJK - not the English-only `text_en`. (Fixes ANW-1178.)
+- **`identifier_match`** (a `WordDelimiterGraphFilter` chain with `catenateAll=1`) generates concatenated token variants at index time, so `MS-2024-001`, `MS 2024 001`, and `MS2024001` all match each other. (Fixes the identifier related issues.)
+- **`string_punct_stop`** treats `& : ; [ ]` as query-side stopwords where appropriate. (Fixes ANW-308, ANW-1686.)
+- **`mm=4<90%`** (minimum-should-match: 1-4 terms all required, 5+ terms 90% required) so short queries stop returning weak any-term matches. (Fixes ANW-859.)
+
+Each of these is a small, localized change once there is a named field to attach the analyzer to - which Principle 1 provides.
+
+### Principle 4: Field names follow a dynamic-field suffix convention.
+
+We adopt Blacklight/ArcLight's convention where a field-name suffix encodes its Solr properties (type, stored, indexed, multivalued), so a new field needs no per-field `<field>` declaration in the solr schema - one `<dynamicField>` pattern covers every field carrying that suffix:
+
+- `*_tesim` = text, stored, indexed, multivalued (full-text-searchable note bodies, titles).
+- `*_ssim` = string, stored, indexed, multivalued (untokenized facet values, e.g. `published_creators_ssim`).
+
+The post-refactor schema has **no static `<field>` declarations for record content** - only dynamic patterns, the unique key, and field-type definitions. Whether a field is searchable, sortable, or multivalued is made explicit by its name suffix.
+
+We **extend** the Blacklight suffix dictionary with a few custom suffixes for analyzer chains that already exist in ArchivesSpace and that no existing ArcLight suffix expresses (the whitespace-tokenizer chain, the ICU sort chains, the identifier-matching chain).
+
+A thin translation layer in the backend search controller maps the renamed fields back to their bare names (`title_tesim` to `title`) in `/search` responses, so external API callers, PUI ERB partials reading `result['title']`, and saved-search URLs keep working unchanged.
+
+### Principle 5: Publish-scoping becomes an explicit per-field discipline.
+
+This is the principle with the most operational risk, and the one we most want careful examination / feedback on.
+
+ArchivesSpace protects unpublished content from public search in two layers:
+
+- **Layer 1 (record-level):** `fq=publish:true` on every PUI request. A record whose `publish` flag is false never appears. **Unchanged by this work.**
+- **Layer 2 (sub-record-level):** the tree-walker diverts subtrees whose `publish` flag is false into a separate queue and emits a published-only `fullrecord_published`. This is what stops a draft `bioghist` note on an otherwise-published resource from matching a PUI keyword. The walker does this *type-blind*: notes, linked-agent relationships, file versions, instances, external documents, rights statements, and anything else carrying a `publish` flag, with no per-type code.
+
+Removing the catchall removes the tree-walker, so **Layer 2 has to be reconstructed explicitly.** The discipline:
+
+> Any field whose value derives from a sub-record structure carrying a per-instance `publish` flag must emit two variants: a published-content field and an unpublished-content field. Requests comming from the PUI (relevance groups, advanced-search whitelist, facets, highlight fields) enumerate only the published variants. Requests from the SUI may use the unpublished variants. The API backend will ensure this policy is enforced. As in the current architecture, neither the SUI nor the PUI will have direct access to solr. They will call backend API endpoints that will forward the requests to solr.
+
+Concretely, the per-note-type emitter splits by publish flag:
+
+```ruby
+# Each note type emits a published field and an unpublished companion.
+# PUI relevance lists only *_tesim; staff may add *_unpublished_tesim.
+doc['bioghist_tesim']             = published_notes_of_type(record, 'bioghist')
+doc['bioghist_unpublished_tesim'] = unpublished_notes_of_type(record, 'bioghist')
+```
+
+The same split applies to creators (`published_creators` for the PUI Creator facet) and to every other sub-record type the walker filters today.
+
+#### What makes this safe rather than a regression waiting to happen.
+
+A **publish-scoping regression spec** carries a published resource containing an unpublished sub-record with a unique marker string. The spec asserts a PUI keyword search for that marker returns zero results. It passes today via the walker; it must keep passing after the walker is removed, via the per-field discipline. Any emitter that forgets the published/unpublished split fails this spec on the first PR that touches its field family. We write this spec *first*, against current code, so it is green from the start and turns red the moment the discipline is violated.
+
+#### The trade-off
+
+This replaces one type-blind mechanism with a per-field obligation that core and plugin developers must maintain. The alternative would be to use solr block-join: reifying each publish-flag-carrying sub-record as a Solr child document and filtering with a block-join. This would restore a single mechanism, but block-join is potentially a much larger change (possibly out of scope here) and lacks a direct ArcLight precedent for publish-aware child filtering. **We would value your read on whether the per-field discipline is maintainable, or whether the block-join route should be brought forward.**
+
+### Principle 6: The JSON display blob is retained. The presentation layer is not touched.
+
+ArchivesSpace's public UI is a **Solr-read application**: it renders result lists and show pages by deserializing a stored `json` blob carried on every Solr document (`public/app/models/record.rb`), reading record content exclusively through `/search` and `/search/records`. About 480 lines of `record.rb` and its subclasses read `@json[...]` directly.
+
+This is where we **deliberately diverge from ArcLight**, which has no blob and uses individual stored display fields. Rewriting that presentation layer would be a large, separate effort with its own risks. Therefore, this work changes only the *relevance query and the searchable index fields*. New searchable fields are added **alongside** the blob, not as a replacement for it. The blob stays `indexed="false"`; display behaviour is unchanged.
+
+It is fine to touch presentation code where a search feature requires it (rendering a highlighted snippet, for instance). What is out of scope is dropping the blob and rewriting record display.
+
+A consequence worth stating plainly for operators: **the tree views (Collection Overview and Collection Organization) are unaffected.** Tree documents never used the walker and are fetched by URI lookup, not relevance query, so none of this changes them. They are covered by regression specs regardless.
+
+### Principle 7: Characterization tests first. Lock current behaviour before changing it.
+
+Search behaviour today is under-specified by automated tests, and some of it is behaviour that real sites depend on in ways no one has written down. So the first phase writes [characterization tests](https://lassala.net/2026/02/09/characterization-tests-a-way-into-legacy-code/) that *assert what the code does today* without changing it: which Solr fields each record type populates, which strings the walker routes to published vs. unpublished, which `@json` keys the PUI reads, and what `qf` / `pf` / `mm` / `fq` each search entry point sends to Solr.
+
+These specs are the safety net. They pin current behaviour so that every later change is measured against a known baseline, and the regression specs (publish-scoping, keyword findability, tree rendering) guard every subsequent PR in CI rather than relying on manual inspection.
+
+---
+
+## 3. Scope boundaries
+
+What this work **does**:
+
+- removes the catchalls and the walker
+- moves relevance config into named Solr paramsets
+- adds per-field analyzers (ICU, identifier matching, punctuation handling, `mm`)
+- migrates the schema to dynamic-field naming with a compatibility translation layer
+
+What this work **explicitly does not do**:
+
+- **Drop the JSON display blob or rewrite the PUI display path.** Retained as-is.
+- **Migrate the hierarchy to Solr block-join nested documents** (`_root_` / `_nest_parent_`).
+
+One operational assumption we want to flag for site operators: **a full Solr reindex will be required** when upgrading to the release that includes this work.
+
+---
+
+## 4. Why ArcLight is the reference (and where we part from it)
+
+[ArcLight](https://github.com/projectblacklight/arclight) is Blacklight-based discovery for EAD-described archival material - the closest active reference project. We borrow its **search-side** patterns and explicitly reject its **display-side** architecture.
+
+**Adopted:** per-field enumeration instead of a catchall (we go further and drop the catchall entirely); the `identifier_match` field type; ICU analyzers; named paramsets invoked via `useParams`; `mm`; the dynamic-field suffix convention.
+
+**Not adopted:** ArcLight's individual stored display fields and no-blob architecture. ArchivesSpace keeps its `json` display blob because the public UI is built around it (Principle 6).
+
+The differences that matter: ArcLight indexes EAD XML via Traject; ArchivesSpace indexes resolved JSON via `IndexerCommon`. ArcLight is read-only on Solr; ArchivesSpace's frontend writes through the backend. ArcLight has no plugin hook surface; **ours must be preserved** - the indexer's `add_indexer_initialize_hook` / `add_document_prepare_hook` / `add_extra_documents_hook` API stays, and the characterization specs assert those hooks still fire on every record.
+
+---
+
+## 5. What this means for plugin authors and operators
+
+This is the part most likely to affect you directly, and where we want a reality check.
+
+- **Plugins that write into `fullrecord` / `fullrecord_published` as the canonical "make this searchable on Staff UI only" \ "make this searchable on the PUI" pattern will need to change.** Today a plugin that wants custom searchable content writes it into the `fullrecord` / `fullrecord_published` catchall. After this work there is no catchall; a plugin emits a named field and that field is added to a relevance group. We need to make this migration path clear and document it. **If your plugin does this, we want to hear how.**
+- **Relevance tuning moves from "edit Ruby and redeploy" to "edit `solrconfig.xml` and reload."** For operators who tune relevance, this should be a net improvement, but it relocates where the configuration lives.
+- **Field names change.** The translation layer keeps `/search` responses speaking the old field names, but anything reading Solr directly (rather than through the API) will see dynamic-suffixed names. **If you query Solr directly, tell us.**
+- **A full reindex is required on upgrade.**
+
+---
+
+## 6. Questions for reviewers
+
+These are the decisions where outside experience would change our confidence most:
+
+1. **The publish-scoping discipline (Principle 5).** Is a per-field published/unpublished obligation, guarded by a regression spec, maintainable in your judgement? Or is the type-blind walker's safety worth enough that we should bring the block-join approach forward despite its cost?
+2. **Removing the catchall entirely (Principle 1), going further than ArcLight.** Do you rely on `fullrecord`-style "search everything" behaviour in a way an explicit allow-list would break? Are there legitimate searches that only work *because* the index is type-blind?
+3. **Plugin impact (§5).** If you maintain a plugin that writes searchable content, what does it write, and would emitting a named field instead be a problem?
+4. **Direct Solr consumers.** Does anything in your deployment read ArchivesSpace's Solr fields by name, outside the `/search` API? The translation layer protects the API but not direct Solr access.
+5. **Relevance defaults.** The `qf` weights and `mm=4<90%` are starting points. If you have tuned ArchivesSpace relevance in production, what did you change and why?
+
+The detailed implementation plan (phasing, sprint-sized tickets, and a field-by-field Solr cross-reference) exists separately and can be shared with anyone who wants to review at that level.