New searchindex behaviour introduced with 4.3.0 seems to have broken searching

[ViktorHaag]

A primary reference here is PR #9649 .

The goal there, as I understand it, was to allow for several identically named objects appearing in different domains and for searchindex.js to find all those results.

The solution in the PR was to change the underlying data format in searchindex.js, switching from a Dict whose values are Dicts (of "name: Tuple of object info") to a Dict whose values are Lists (of items which are "Tuple of object info, now including name as the last element").

This seems to have changed the way that search results are presented in the list.

Here are some screen shots of my docs so you can see the difference.

The first list of search items for a search term is generated with Sphinx 4.2.0:

The second list of search items for a search term is from the same documentation source, rebuilt to a fresh output directory with Sphinx 4.3.1 (I observed the same result as this with Sphinx 4.3.0 as well):

In the source, I use the js domain to document the JSON structures for my REST API; for example, the dos for the Exemptions.ItemExemptionInfoFilter looks like this ("d2ljson" is a special enhanced version of JSON that allows me to document JSON showing type info, comments, and so on -- the lexing of the code block should have no effect at all on the JS attribute object; it's just content for that documentation item):

.. js:attribute:: Exemptions.ItemExemptionInfoFilter

    .. code-block:: d2ljson

       {
          "ExemptStartDate": <string:UTCDateTime>|null,
          "ExemptEndDate": <string:UTCDateTime>|null,
          "ExcludeGradedActivities": <boolean>|null,
          "ExcludeCompleteActivities": <boolean>|null
       }

In the first example where we used the old search method, searching presented back a search hit for Exemptions.ItemExemptionInfoFilter that went to the right place in the docs. For the second example with the new search method, it looks like the presentation of the search result is return the index of the part of the JS attribute name, and not the name of it ("1" vs "ItemExemptionInfoFilter").

Additionally, in the first example where we used the old search method, search hits on HTTP routes (API calls in my REST API) result. In the second example with the new search method, those search hits are completely missed. To document the HTTP routes, I use a slightly modified version of the sphinxcontrib-httpdomain extension. Code for a simple HTTP route API call looks like this:

.. http:post:: /d2l/api/le/(version)/(orgUnitId)/content/exemptions/users/(userId)/byFilter/
   :scopes: content:exemptions:write

   Create exemptions for content topics matching a specific filter for a user.

   :param version: API version.
   :type version: :term:`D2LVERSION`
   :param orgUnitId: Org unit ID.
   :type orgUnitId: :term:`D2LID`
   :param userId: User for whom to create the exemptions.
   :type userId: :term:`D2LID`
   :jsonparam ItemExemptionInfoFilter: Filter to select content items for exemption.
   :jtype ItemExemptionInfoFilter: :js:attr:`ItemExemptionInfoFilter <Exemptions.ItemExemptionInfoFilter>`
   :status 200: Action succeeded.
   :status 403: No permission to create exemptions
   :status 429: API :ref:`call-rate limit <rate-limiting>` exceeded.
   :since 1.47+: Route first appears in LMS v20.20.8.

   **Returns**. This action returns the created exemptions expressed in a
   :js:attr:`ItemExemptionInfo <Exemptions.ItemExemptionInfo>` JSON block.

You can see that this API route is found in the first set of search results produced with the 4.2.0 build, but not in the second set of search results from the 4.3.1 build.

I'm a bit of a loss as to how to achieve the goal of the original PR, but it seems clear to me that the PR is likely not working entirely as intended, and I'd request that we revert it until we have a more robust solution with more tests that can avoid results like above.

I'll try to provide more info or assistance here if I can, but I'm not sure what do do next.

How to Reproduce

Steps are very simple: do an HTML build with Sphinx 4.2.0 to show the original search results that present as I expect; then do an HTML build with Sphinx 4.3.1 to show the less useful search results.

Expected behavior

The presentation of search results should not change despite the changes in PR #9649 which should permit for new search results if the same object name gets used in several domains.

Your project

Unable to provide my own project as the documentation is internal.

Screenshots

Screen shots in description.

OS

MacOS 12.0.1

Python version

3.10.0

Sphinx version

4.3.1

Sphinx extensions

Our build uses a number of local extension modules; none directly to anything with Sphinx's search functionality. Our local 'httpdomain' and 'phpdomain' modules are wrappers/enhancements of the sphinxcontrib-provided domains.

    "sphinx.ext.autodoc",
    "sphinx.ext.extlinks",
    "sphinx.ext.intersphinx",
    "sphinx.ext.todo",
    "d2l_local",
    "d2l_htmlbuilder",
    "d2l_httpdomain",
    "d2l_phpdomain",
    "d2l_pygments",

Extra tools

No response

Additional context

No response

[ViktorHaag]

I wonder if we tried to keep the old data structure of a dict of dicts, but changed the way we put items into that top level dict to incorporate the domain name as well as the prefix as the first-level key?

Right now, in 4.2.0's version of IndexBuilder.get_objects (starting at line 311) we do this to populate the top-level entries in the search index; the keys are the "prefix" of the various display names of the objects from all the domains:

                fullname = html.escape(fullname)
                dispname = html.escape(dispname)
                prefix, _, name = dispname.rpartition('.')
                pdict = rv.setdefault(prefix, {})

What if we did something like this instead, to incorporate the domain name into the key value as well?

                fullname = html.escape(fullname)
                dispname = html.escape(dispname)
                nameprefix, _, name = dispname.rpartition('.')
                prefix = "{}-{}".format(domainname, nameprefix)
                pdict = rv.setdefault(prefix, {})

Later code using the search index would need to potentially know to pick apart the key name of prefix later on into a domain name + a name prefix if it needed to. But it would keep the same overall datastructure in place of a dict of dicts?

[tk0miya]

@jakobandersen Could you take a look at this, please? Is it better to revert #9649 to recovery?

[jakobandersen]

@tk0miya, until the problem is confirmed I think it is premature to revert the PR.

It's a bit difficult to reproduce from the description, but I tried the following:
index.rst:

.. js:attribute:: Exemptions.ItemExemptionInfoFilter

and then with no extensions at all. If I understand correctly this is one of the items that is supposedly wrong. However, when I search for exemption, then I get the result

Exemptions.ItemExemptionInfoFilter (JavaScript attribute, in <no title>)

which seems fine to me. I tried both with v4.2.0, v4.3.2, and 4.3.x with the same result.
@ViktorHaag, could it be a stale objects.inv searchindex.js or something? Otherwise, can you try to make a minimal example with only publicly available extensions to more easily pin down the problem?

[tk0miya]

Okay, I move this to the 4.5.0 milestone.

[ViktorHaag]

@jakobandersen Thanks for investigating; it's not a stale searchindex.js since as part of testing I regularly clean/erase the build output directory before rebuilding from scratch. I will investigate further on my end to see if I can reproduce with as stock an environment as possible. I couldn't see how my custom extensions would have had an effect on this, but perhaps they have. Looks like I have a lot of work ahead of me this week.

I now have a strong suspicion that for some reason in the distant past I had put a copy of Sphinx' searchtools.js into my local project theme's static directory, and while I was regularly rebuilding searchindex.js it seems like me using an outdated version of searchtools.js was almost certainly the problem. This would certainly explain why starting a new project from scratch seems unable to replicated this defect.

I'm going to do more testing to confirm this suspicion.

[ViktorHaag]

I am now convinced that the real problem here is my dependency on an outdated copy of searchtools.js, so I am closing this issue. Apologies folks for the time you've spent on this. @tk0miya I feel confident you can remove this from the 4.5.0 milestone.

[jakobandersen]

I am now convinced that the real problem here is my dependency on an outdated copy of searchtools.js, so I am closing this issue. Apologies folks for the time you've spent on this. @tk0miya I feel confident you can remove this from the 4.5.0 milestone.

No worries, when the change was made we briefly discussed what it may break and concluded it would be unlikely it would break anything. So if you happen to find out the source of getting searchtools.js out of sync, that would indeed be valuable knowledge.

[ViktorHaag]

No worries, when the change was made we briefly discussed what it may break and concluded it would be unlikely it would break anything. So if you happen to find out the source of getting searchtools.js out of sync, that would indeed be valuable knowledge.

Checking the version history for my project it seems as if waaaaay back in the Sphinx 1.8/Sphinx 2 timeframe, I ran into problems with the newer Sphinx version not handling searching in my project and my solution was to grab the older searchtool.js and vendorize it in my project.

I think that the challenges had something to do with changes in the htmlToText function inside the searchtools code, but it's not clear exactly what was troublesome.

At any rate, whatever was causing the older historical problems seems to have been fixed in the meantime, so now just using the stock components seems to be working just fine (and not using the default provided code was causing my problem).