RFC: search refactoring principles and direction#1
Conversation
Proposes replacing ArchivesSpace's single catchall Solr field with named, allow-listed fields, and moving relevance tuning out of Ruby into Solr config. Sets out guiding principles and open questions for reviewers. This is a request for feedback on the direction (not the implementation plan) ahead of a large structural change to search. Related: ANW-2229 (roadmap ASRM-27).
thimios
changed the title
|
|
||
| #### 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.** |
There was a problem hiding this comment.
Just throwing in a possible third option for consideration. The type-blind mechanism could be added back to the PUI indexer (pui_indexer.rb) in a more systemic way by overriding add_document_prepare_hook:
def add_document_prepare_hook(&block)
super do |doc, record|
block.call(doc, strip_unpublished(record))
end
end
Where strip_unpublished would do a tree walk and drop any obj where obj['publish'] == false. This would mean that the PUI mapping rules could be written to assume any data they can access is published.
There was a problem hiding this comment.
Thanks @marktriggs, sure I see this as a smart safety net, where non-published objects are not passed over to the logic that prepares the solr document for the record. This safety mechanism could be used regardless of whether we keep the fullrecord / fullrecord_published fields or not.
Do you @marktriggs imply that it would be better to keep the type-blind fullrecord / fullrecord_published fields in any case? I am super interested in arguments that support the idea of keeping these fields.
There was a problem hiding this comment.
@thimios Yeah, I see the safety mechanism discussion as separate from the fullrecord/separate fields discussion. I just like the safety mechanism because I'm exactly the sort of person who will forget to filter out the non-published stuff when writing PUI mappings ;)
I'm not really advocating for the fullrecord/fullrecord_published fields, and I think your original document did a good job of laying out the trade-offs of that approach versus having more specific fields. Dumping everything into a fullrecord catch-all gets you searches with high recall but lower precision. In the context of the original ArchivesSpace development project, we were pretty time-constrained, and I still think using the catch-all field was a good trade-off for the development time we had available. If a search returns too many results, relevance ranking, facets & sorting can help users to work around that, but if documents just don't turn up in results at all, there's not much users can do about it besides file a bug report.
But I think having the separate fields gets you better quality search results (and avoids the kinds of bugs you listed), as long as you (and plugin authors) have the time to do the more fine-grained work of mapping at index time. And I think it's reasonable to choose different trade-offs at different points in the system's lifecycle.
|
|
||
| ### 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: |
There was a problem hiding this comment.
Would the existing set of dynamic field suffixes stick around under the new regime? Most of the plugins we've written make use of the existing set of *_u_* suffixes.
There was a problem hiding this comment.
Proposed suggestion regarding this: could we retain the existing *_u_* prefixes but deprecate them noting that they will be fully removed in a future version (e.g. ArchivesSpace 5.0.0)?
There was a problem hiding this comment.
Great point. I also see it as @anarchivist proposed: We retain the existing prefixes with deprecation warnings until a future version.
We will also provide a table with mappings from the current dynamic fields to the proposed new ones:
| Current suffix | Properties | New suffix |
|---|---|---|
| *_enum_s | string, stored, indexed, multi | *_ssim |
| *_u_ustr | string, unstored, indexed, multi | *_sim |
| *_u_sstr | string, stored, indexed, multi | *_ssim |
| *_u_uint | int, unstored, indexed, multi | *_iim |
| *_u_sint | int, stored, indexed, multi | *_isim |
| *_u_utext | text, unstored, indexed, multi | *_teim |
| *_u_stext | text, stored, indexed, multi | *_tesim |
| *_u_udate | date, unstored, indexed, multi | *_dtim |
| *_u_sdate | date, stored, indexed, multi | *_dtsim |
| *_u_ubool | boolean, unstored, indexed, multi | *_bim |
| *_u_sbool | boolean, stored, indexed, multi | *_bsim |
| *_u_sortdate | date, unstored, indexed, single | *_dti |
| *_u_ssortdate | date, stored, indexed, single | *_dtsi |
| *_int_sort | int, unstored, indexed, single | *_ii |
| *_u_typeahead_utext | text_general, stored, indexed, single | *_tesi |
Notice that *_u_utext / *_u_stext are typed text_general, which we want to move away from (see Principle 3)
During the deprecation window those legacy fields could keep the old analysis behavior. A plugin would get the new ICU analysis by renaming to one of the new suffixes.
|
|
||
| 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.** |
There was a problem hiding this comment.
I'm only doing this in a private plugin where I add the resource title and concatted identifier to descendant AOs so that a search for the 'call number' or title of the collection picks up the resource and AOs. Without seeing the actual mechanisms, I think this could be accomplished with the proposed changes.
|
|
||
| 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. |
There was a problem hiding this comment.
I've got a plugin (https://github.com/dartmouth-dltg/aspace_search_modifications) that modifies the fields that are added to the fullrecord so that a configurable list of fields can be omitted from the fullrecord. Will this be possible in this new proposal without altering the solr schema? IE will a plugin be able to modify the proposed allow list of searchable fields?
|
|
||
| - **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.** |
There was a problem hiding this comment.
Not sure where to put this, but will we still be able to hook into Solr.add_search_hook to modify queries?
| 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? |
There was a problem hiding this comment.
We've tuned relevance to preference resources over archival objects. Our config looks like
AppConfig[:solr_params] = {
'q.op' => 'AND',
"bq" => ["title:\"#{@query_string}\"*", 'primary_type:archival_object^50', 'primary_type:resource^51'],
"pf" => 'title^10',
"ps" => 0,
"hl" => 'true',
}
|
|
||
| 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? |
There was a problem hiding this comment.
Plugins write numerous custom fields of various types (using the *_u_* fields naming). I don't think emitting a named field would be an issue, but the devil is in the details!
| 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? |
There was a problem hiding this comment.
Mentioned above, but we do push a few additional pieces of info into fullrecord for archival objects. As long as we can add that same information so that results surface in a general search, I'm OK with changing the mechanism.
| - `*_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. |
There was a problem hiding this comment.
Proposed suggestion: can we extend this the same way to include publish-scoping? (I'll make another comment about this below.)
| > 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') | ||
| ``` |
There was a problem hiding this comment.
Continuing from my comment above: is it possible to extend the field-typing suffix scheme to reference the publish flag? What I'm thinking about here specifically are cases where it might easier to incorporate that flag in the field suffix dynamically.
| 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. |
There was a problem hiding this comment.
This is a huge gain IMO. I wonder how we'll treat this from a documentation perspective (tuning relevance is, as we know, quite complex). I think there's a great opportunity to help the community here but I want to be how mindful of how we're best going to support the community.
4 participants
This PR publishes a request for feedback on a proposed structural change to ArchivesSpace search. It is not a code change the only file is the RFC document itself. The PR is meant as a place for the ArchivesSpace community to discuss the principles of the proposed change.
A cluster of long-standing search bugs share one root cause: every field of a record is concatenated into a single catchall Solr field, with relevance rules hard-coded in Ruby.
The RFC document included in this PR proposes replacing the catchall with named, allow-listed Solr fields and moving relevance tuning into Solr config. It presents guiding principles, scope boundaries, why ArcLight is the reference (and where we diverge), and potential impact on plugin authors and operators.
Read the document in a rendered view here:
search-refactoring-principles-rfc.mdWho we're hoping to hear from
People who run ArchivesSpace at scale, maintain a plugin that touches Solr, or have opinions about discovery relevance. Anyone is welcome to comment you don't need write access to this repo.
How to give feedback
+next to any line to comment on a specific principle or sentence.Context