Spellcheck "Did you mean" - search_api_spellcheck

See here for a review of the module in general: #1157404: Module review.
As stated there, this seems to work fine and it's a great start. However, as for the module, I also have a couple of concerns with the patch that should be addressed before I can commit it.

The README.txt needs to be updated regarding the supported third-party features.

+++ includes/SearchApiSpellcheckSolr.inc
@@ -0,0 +1,34 @@
+/**
+ * @file SearchApiSpellcheckSolr.inc
+ *
+ *
+ */

This shouldn't state the file, but have a short description of its contents (i.e., the class).
The class also needs a doc comment.

+++ includes/SearchApiSpellcheckSolr.inc
@@ -0,0 +1,34 @@
+  private function _solrSuggestions(stdClass $suggestions) {

The helper method doesn't seem to be necessary, as it is called only once anyways. Just inline it in the constructor.
You also shouldn't use stdClass as a type hint, but only interfaces (but of course this is moot when the method is removed).

+++ schema.xml
@@ -318,6 +318,11 @@
+   <!-- This field is used to build the spellchecker index -->
+   <field name="spell" type="textSpell" indexed="true" stored="true" multiValued="true"/>
+  <!-- copy all text fields into our spell field for indexing. -->
+   <copyField source="t_*" dest="spell"/>

Comments should use proper punctuation and capitalization.

+++ service.inc
@@ -444,6 +444,9 @@ class SearchApiSolrService extends SearchApiAbstractService {
+    if (isset($options['search_api_spellcheck']) AND $options['search_api_spellcheck'] === TRUE) {

Use "&&", not "AND". But the whole line could also just read if (!empty($options['search_api_spellcheck'])) {.

+++ service.inc
@@ -547,6 +550,11 @@ class SearchApiSolrService extends SearchApiAbstractService {
+    if (module_exists('search_api_spellcheck')) {
+      $results['search_api_spellcheck'] = new SearchApiSpellcheckSolr($response);
+    }

Should check whether the query option was set, not just whether the module exists. (Or, maybe both, since of course you also want to avoid fatal errors due to an unavailable base class.)

+++ solrconfig.xml
@@ -557,7 +557,18 @@
+      <!-- By default, don't spell check -->
+      <str name="spellcheck">false</str>
+      <!-- Defaults for the spell checker when used -->
+      <str name="spellcheck.onlyMorePopular">true</str>
+      <str name="spellcheck.extendedResults">false</str>
+      <!--  The number of suggestions to return -->

Punctuation/capitalization.

Powered by Dreditor.

This looks great. I encountered one issue whilst following your instructions which I’d like to highlight.

Using spellcheck=true at the same time as using using a filter query (fq) directive for me returns no suggestions.

The following queries aim to demonstrate this, they were run against a generated dataset of 50 nodes. The query string in each case was 'test':

Original Call:

spellcheck=true&facet.missing=false&facet=true&facet.mincount=1&facet.limit=10&qf=t_title&qf=t_search_api_fulltext&qf=t_body:value&json.nl=map&wt=json&version=1.2&rows=10&facet.sort=count&start=0&q="test"&facet.field=f_ss_type&fq=index_id:default_node_index

FQ in this context highlights which Search API index to use

Original Response

{"responseHeader":{"status":0,"QTime":5},"response":{"numFound":0,"start":0,"maxScore":0.0,"docs":[]},"facet_counts":{"facet_queries":{},"facet_fields":{"f_ss_type":{}},"facet_dates":{}},"spellcheck":{"suggestions":{}}}

If I remove the Filter query declaration “&fq=index_id:default_node_index “ I receive a more expected result:

{"responseHeader":{"status":0,"QTime":5},"response":{"numFound":0,"start":0,"maxScore":0.0,"docs":[]},"facet_counts":{"facet_queries":{},"facet_fields":{"f_ss_type":{}},"facet_dates":{}},"spellcheck":{"suggestions":{"test":{"numFound":1,"startOffset":1,"endOffset":5,"suggestion":["tego"]}}}}

Running the original query with ‘Spellcheck Collate’ enabled I also receive my expected response:

{"responseHeader":{"status":0,"QTime":5},"response":{"numFound":0,"start":0,"maxScore":0.0,"docs":[]},"facet_counts":{"facet_queries":{},"facet_fields":{"f_ss_type":{}},"facet_dates":{}},"spellcheck":{"suggestions":{"test":{"numFound":1,"startOffset":1,"endOffset":5,"suggestion":["tego"]},"collation":"\"tego\""}}}

Collation appears to has resolved the issue, the documentation hints at a potential reason:

spellcheck.collate can guarantee that collations will return results if re-run by the client (applying original fq params also). This is especially helpful when there is more than one correction per query. There is also an option to get multiple collation suggestions and an expanded response format. The following three parameters enable this function

See: http://wiki.apache.org/solr/SpellCheckComponent#spellcheck.collate

I have checked the collate option against a search which doesn’t have a filterquery and the results did not appear to be adversely affected.

My Solr knowledge is too limited to advise that Collation is a solution, however it has worked for me so may be a worthwhile avenue to explore.

Quite good already, attached are some additional fixes. With those, I'd say this is ready to be committed. Although some feedback from others (and from you) would be nice before that.

@ Darren: Couldn't reproduce this, for me this works with or without filter queries. (Also, filter queries will always be used in Solr queries, due to filtering by index – ergo, this otherwise would never work.) Don't really know what could be wrong on your side. (Maybe "test" and "tego" are just too far away from each other to be reliably found? Try to test with something that only misses one letter – e.g., I use "mact" for my devel-generated data.)

I only found one issue with filter queries, namely that they aren't considered when testing whether the corrected term has more results. This could result in you clicking the "Did you mean: …" link and ending up with less results than before, or none at all. However, for normal spellchecking (I tried it with test data) this will rarely be the case, I guess. Also, as far as I know, Google also doesn't get this right – and who better to measure against than them? ;)

@drunken monkey - Thankyou for looking into my particular issue. Following your advice I have since tried to replicate and this is no longer reproducible on my setup. You may be correct in your theory that the distance between "tego" and "test was causing the hit/miss behaviour.

I have removed the collation from my setup and it works fine.

Oh, you are right, good catch!
I just tried this out, see the attached patch. The primitive approach of just adding the necessary settings to both request handlers seems to work fine. (To test, change service.inc:398 to $this->request_handler = 'standard';.)
Does this work for you, too?
And would you therefore say this can be commited?

I have tested Drunken Monkeys latest patch alongside release 1.0 of search_api_spellcheck and the dev release of search_api and this works well for me. There also appear to be no adverse affect of including this patch without having search_api_spellcheck enabled.

For anyone interested in testing this patch, I am currently using the following in my make file. You will need the latest version of drush make with the patch to allow p1 patching:

; Search API - Running off a commit of dev branch until http://drupal.org/node/1153298 makes it into a tag
projects[search_api][version] = 1.x-dev
projects[search_api][subdir] = "drupal.org"
projects[search_api][download][type] = get
projects[search_api][download][url] = "http://drupalcode.org/project/search_api.git/snapshot/b425730.tar.gz"

; Search API Solr requires latest dev version and a patch
projects[search_api_solr][version] = 1.x-dev
projects[search_api_solr][subdir] = "drupal.org"
projects[search_api_solr][download][type] = get
projects[search_api_solr][download][url] = "http://drupalcode.org/project/search_api_solr.git/snapshot/e478c1f.tar.gz"

; Patch http://drupal.org/node/1153306
projects[search_api_solr][patch][0][url] = "http://drupal.org/files/issues/search_api_solr.spellcheck_0.patch"
projects[search_api_solr][patch][0][strip] = 1
projects[search_api_solr][patch][0][target] = ""

; Get PHP library required by Search API Solr
libraries[SolrPhpClient][download][type] = get
libraries[SolrPhpClient][download][url] = http://solr-php-client.googlecode.com/files/SolrPhpClient.r22.2009-11-09.zip
libraries[SolrPhpClient][directory_name] = SolrPhpClient
libraries[SolrPhpClient][destination] = modules/drupal.org/search_api_solr

projects[search_api_spellcheck][version] = "1.0"
projects[search_api_spellcheck][subdir] = "drupal.org"

Excellent!
Committed this now, thanks again!