Merge remote-tracking branch 'origin/main' into wilm

shopdb/core/api/assets.py

@@ -506,6 +506,17 @@ def update_asset(asset_id: int):
     if not data:
         return error_response(ErrorCodes.VALIDATION_ERROR, 'No data provided')
 
+    # An UPDATE may not blank the asset number. Create validates it and the
+    # column is NOT NULL, but the conflict check below only runs when the value
+    # DIFFERS, and '' never collides with anything - so a payload carrying an
+    # empty assetnumber wrote it straight through. A form that loaded blank
+    # (a failed reference load, a partial fetch) then saved the blank over a
+    # real record.
+    if 'assetnumber' in data and not (data['assetnumber'] or '').strip():
+        return error_response(
+            ErrorCodes.VALIDATION_ERROR,
+            'assetnumber cannot be empty')
+
     # Check for conflicting assetnumber
     if 'assetnumber' in data and data['assetnumber'] != asset.assetnumber:
         if Asset.query.filter_by(assetnumber=data['assetnumber']).first():

shopdb/core/api/search.py

@@ -9,13 +9,14 @@ from flask import Blueprint, request, current_app
 from flask_jwt_extended import jwt_required
 from sqlalchemy.orm import joinedload
 
-from shopdb.extensions import db
+from shopdb.extensions import db, cache
 from shopdb.core.models import (
     Application, Setting,
     Asset, AssetType, Communication, Vendor, Model,
     CustomField, CustomFieldValue
 )
 from shopdb.core.api.settings import get_cached_settings
+from shopdb.core.services.spellfix import Vocabulary
 from shopdb.utils.responses import success_response
 
 logger = logging.getLogger(__name__)
@@ -31,7 +32,7 @@ search_bp = Blueprint('search', __name__)
 # cannot reorder. Relevance is applied afterwards in Python, over a stable set.
 
 
-def _word_match(query, *columns):
+def _word_match(query, *columns, extra=None):
     """SQL clause matching rows that contain EVERY word of the query, each word
     in any of the columns, in any order.
 
@@ -39,14 +40,25 @@ def _word_match(query, *columns):
     multi-word search returned nothing. This splits the query into words and ANDs
     them (OR across the columns per word): 'CSF Roles' matches a row with 'CSF'
     and 'Roles' anywhere in the searched fields.
+
+    `extra` is an optional callable taking one word and returning a further
+    clause to OR in for that word. Use it where a word may be satisfied by a
+    RELATED row rather than a column of this table - a knowledge base article
+    matching on its topic's name, say. Splitting per word matters there: without
+    it a word can only ever be matched by the columns, so a query naming the
+    topic and a keyword ('CMM Community') matches neither side alone.
     """
     words = [w for w in query.split() if w]
     if not words:
         words = ['']
-    return db.and_(*[
-        db.or_(*[c.ilike(f'%{w}%') for c in columns])
-        for w in words
-    ])
+
+    def per_word(word):
+        clauses = [c.ilike(f'%{word}%') for c in columns]
+        if extra is not None:
+            clauses.append(extra(word))
+        return db.or_(*clauses)
+
+    return db.and_(*[per_word(w) for w in words])
 
 
 def _require_enabled(name):
@@ -253,6 +265,10 @@ def _search_knowledgebase(query, search_term):
     subject, which reads as though it were still in service.
 
     A null topic still matches. Not every article is about an application.
+
+    The topic's NAME is searched too, per word. An article tagged 'community'
+    under the CMM topic has neither word in both of its own columns, so a search
+    for 'CMM Community' found it in neither the plugin's listing nor here.
     """
     results = []
     try:
@@ -260,12 +276,24 @@ def _search_knowledgebase(query, search_term):
         from plugins.knowledgebase.models import KnowledgeBase
         retired = db.session.query(Application.appid).filter(
             Application.isactive.is_(False))
+
+        def topic_named(word):
+            """Articles whose topic is an ACTIVE application named like word.
+
+            Active only, for the same reason the retired filter above exists: a
+            decommissioned application is not a topic anyone should be offered.
+            """
+            return KnowledgeBase.appid.in_(
+                db.session.query(Application.appid).filter(
+                    Application.appname.ilike(f'%{word}%'),
+                    Application.isactive.is_(True)))
+
         kb_articles = KnowledgeBase.query.filter(
             KnowledgeBase.isactive == True,
             db.or_(KnowledgeBase.appid.is_(None),
                    KnowledgeBase.appid.notin_(retired)),
             _word_match(query, KnowledgeBase.shortdescription,
-                        KnowledgeBase.keywords)
+                        KnowledgeBase.keywords, extra=topic_named)
         ).order_by(KnowledgeBase.clicks.desc(),
                    KnowledgeBase.linkid).limit(20).all()
 
@@ -976,6 +1004,94 @@ def _check_smart_redirect(query, classification):
     return None
 
 
+# How long a harvested vocabulary is reused. Long enough that a burst of
+# searching does not rebuild it, short enough that something entered this
+# morning is correctable before lunch.
+_VOCABULARY_TTL = 600
+
+# Per-column ceiling on harvested terms. A correction is only offered when a
+# search found NOTHING, so the vocabulary exists to catch a typo against the
+# names people actually use - not to mirror the database. Without a bound this
+# would load every asset number on a site into memory to answer one query.
+_VOCABULARY_LIMIT = 5000
+
+
+def _search_vocabulary():
+    """Words worth correcting a search against, harvested from the data.
+
+    From the DATA and not a dictionary, because no dictionary holds Genspect,
+    Telesis, Keyence or wax-trace - and because a vendor added this morning is
+    then correctable this morning.
+
+    Cached: this runs only on a search that found nothing, but a person who
+    mistypes once usually mistypes twice.
+    """
+    cached = cache.get('search_vocabulary')
+    if cached is not None:
+        return cached
+
+    vocabulary = Vocabulary()
+    columns = (
+        (Asset, (Asset.assetnumber, Asset.name)),
+        (Vendor, (Vendor.vendor,)),
+        (Model, (Model.modelnumber,)),
+        (Application, (Application.appname,)),
+        (AssetType, (AssetType.assettype,)),
+    )
+    for model, fields in columns:
+        for field in fields:
+            try:
+                rows = (db.session.query(field)
+                        .filter(field.isnot(None))
+                        .distinct()
+                        .limit(_VOCABULARY_LIMIT)
+                        .all())
+            except Exception:
+                # A plugin table missing on a lean install must not break
+                # search. No vocabulary is worse than the alternative, not
+                # fatal.
+                logger.debug('vocabulary: skipped %s', field, exc_info=True)
+                continue
+            for (value,) in rows:
+                vocabulary.add(value)
+
+    cache.set('search_vocabulary', vocabulary, timeout=_VOCABULARY_TTL)
+    return vocabulary
+
+
+def _suggestion_for(query):
+    """A better spelling of `query`, or None. Never raises into a search."""
+    try:
+        return _search_vocabulary().suggest_query(query)
+    except Exception:
+        logger.warning('vocabulary lookup failed', exc_info=True)
+        return None
+
+
+@search_bp.route('/suggest', methods=['GET'])
+@jwt_required(optional=True)
+def search_suggest():
+    """A better spelling for a query that found nothing, or null.
+
+    ITS OWN ENDPOINT, deliberately, rather than a `suggestion` key added to
+    every list response. Thirty routes take a search parameter across a dozen
+    files, so folding it into each one is a large change today and a thing every
+    future plugin has to remember. A page that finds no rows calls this instead;
+    a page that has not adopted it simply shows no suggestion, which is what it
+    showed before.
+
+    Returns null rather than a poor guess. The caller offers "did you mean" and
+    a person decides - nothing searches on their behalf.
+    """
+    query = request.args.get('q', '').strip()
+    if not query or len(query) < 2 or len(query) > 200:
+        return success_response({'query': query, 'suggestion': None})
+    return success_response({
+        'query': query,
+        'suggestion': _suggestion_for(query),
+    })
+
+
 @search_bp.route('', methods=['GET'])
 @jwt_required(optional=True)
 def global_search():
@@ -1072,6 +1188,21 @@ def global_search():
 
     total_all = len(unique_results)
 
+    # A search that found NOTHING is the only evidence of what people cannot
+    # find, and it is not otherwise recorded anywhere. Logged at INFO with a
+    # stable prefix so a week of it can be grepped into a list, which is what
+    # should decide whether spelling correction is worth wiring in - the recent
+    # multi-word and cross-field matching changes may already have fixed a good
+    # share of what used to fail. Guessing at that would be building for a
+    # problem nobody has measured.
+    suggestion = None
+    if not unique_results:
+        logger.info('search-no-results query=%r', query)
+        # Offered, never applied. The response carries the better spelling and
+        # the caller decides: silently searching for something else is how
+        # somebody orders the wrong cartridge.
+        suggestion = _suggestion_for(query)
+
     # Limit total results
     unique_results = unique_results[:50]
 
@@ -1083,6 +1214,8 @@ def global_search():
         'total_all': total_all,
         'counts': type_counts,
     }
+    if suggestion:
+        response_data['suggestion'] = suggestion
 
     redirect = _check_smart_redirect(query, classification)
     if redirect:

shopdb/core/services/spellfix.py

@@ -0,0 +1,169 @@
+"""Suggest a spelling for a search word that found nothing.
+
+WHY NOT SOUNDEX. MySQL has it; SQLite does not, and the suite runs on SQLite
+while production runs MySQL - so a SOUNDEX() in a query is either an error in
+every test or a production path no test ever executes. It is also wrong for this
+data: soundex is English-name phonetics, four characters wide, and it DISCARDS
+DIGITS, so CSF16 and CSF17 hash identically. Half of what people search here is
+an identifier.
+
+So this is character-based, runs in Python, and behaves the same on both
+dialects.
+
+THE VOCABULARY IS THE DATA. No dictionary knows Genspect, Telesis, Keyence or
+wax-trace. The terms come from the columns being searched, which also means the
+corpus is never stale: a vendor added this morning is correctable this morning.
+
+IT SUGGESTS, IT DOES NOT REWRITE. The caller offers "did you mean" on a search
+that found nothing. Silently searching for something else is how somebody orders
+the wrong cartridge.
+"""
+
+from collections import defaultdict
+
+# Longest edit distance allowed, by length of the word being corrected. A short
+# word has few ways to be wrong, so one edit is already a different word: 'bay'
+# to 'bar' is not a typo anybody wants guessed for them.
+_DISTANCE_BY_LENGTH = ((4, 1), (8, 2))
+_MAX_DISTANCE = 3
+
+# Below this a word is too short to correct at all.
+MIN_LENGTH = 3
+
+# Candidates sharing no trigram with the query are not scored. Cuts the work
+# from the whole vocabulary to a handful without changing the answer, because a
+# word within the distances above always shares at least one trigram.
+_TRIGRAM = 3
+
+
+def _digits(word):
+    return ''.join(c for c in word if c.isdigit())
+
+
+def max_distance(word):
+    for length, allowed in _DISTANCE_BY_LENGTH:
+        if len(word) <= length:
+            return allowed
+    return _MAX_DISTANCE
+
+
+def damerau_levenshtein(first, second, cutoff=None):
+    """Edit distance counting a transposition as ONE edit, not two.
+
+    Transposition is the commonest typing error - 'Keyecne' for 'Keyence' - and
+    plain Levenshtein charges two for it, which pushes real typos past the
+    threshold on short words.
+
+    `cutoff` abandons a comparison that cannot come in under it. Most candidates
+    are hopeless and this is the difference between scoring a vocabulary and
+    scanning one.
+    """
+    if first == second:
+        return 0
+    if abs(len(first) - len(second)) > (cutoff if cutoff is not None else 99):
+        return (cutoff or 99) + 1
+
+    previous = list(range(len(second) + 1))
+    beforelast = None
+    for i, a in enumerate(first, 1):
+        current = [i] + [0] * len(second)
+        for j, b in enumerate(second, 1):
+            cost = 0 if a == b else 1
+            current[j] = min(previous[j] + 1,        # deletion
+                             current[j - 1] + 1,     # insertion
+                             previous[j - 1] + cost)  # substitution
+            if (beforelast is not None and i > 1 and j > 1
+                    and a == second[j - 2] and first[i - 2] == b):
+                current[j] = min(current[j], beforelast[j - 2] + cost)
+        if cutoff is not None and min(current) > cutoff:
+            return cutoff + 1
+        beforelast, previous = previous, current
+    return previous[-1]
+
+
+def _trigrams(word):
+    padded = '  %s ' % word
+    return {padded[i:i + _TRIGRAM] for i in range(len(padded) - _TRIGRAM + 1)}
+
+
+class Vocabulary:
+    """Words harvested from searchable data, indexed for near-match lookup."""
+
+    def __init__(self, terms=()):
+        self.words = set()
+        self._bytrigram = defaultdict(set)
+        for term in terms:
+            self.add(term)
+
+    def add(self, term):
+        for word in str(term or '').split():
+            word = word.strip().lower()
+            if len(word) < MIN_LENGTH:
+                continue
+            if word in self.words:
+                continue
+            self.words.add(word)
+            for gram in _trigrams(word):
+                self._bytrigram[gram].add(word)
+
+    def candidates(self, word):
+        found = set()
+        for gram in _trigrams(word):
+            found |= self._bytrigram.get(gram, set())
+        return found
+
+    def suggest(self, word):
+        """The best correction for `word`, or None to leave it alone.
+
+        Returns None rather than a poor guess: a wrong suggestion costs more
+        than no suggestion, because somebody acts on it.
+        """
+        word = (word or '').strip().lower()
+        if len(word) < MIN_LENGTH or word in self.words:
+            return None
+
+        limit = max_distance(word)
+        best, bestdistance = None, limit + 1
+        for candidate in self.candidates(word):
+            if not _acceptable(word, candidate):
+                continue
+            distance = damerau_levenshtein(word, candidate, cutoff=limit)
+            if distance < bestdistance:
+                best, bestdistance = candidate, distance
+            elif distance == bestdistance and best is not None:
+                # A tie means two equally good answers and no way to choose.
+                # Offering either one implies a confidence that is not there.
+                best = None
+        return best if bestdistance <= limit else None
+
+    def suggest_query(self, query):
+        """Correct a whole query, or None if nothing in it can be improved."""
+        words = [w for w in (query or '').split() if w]
+        if not words:
+            return None
+        fixed, changed = [], False
+        for word in words:
+            suggestion = self.suggest(word)
+            if suggestion and suggestion != word.lower():
+                fixed.append(suggestion)
+                changed = True
+            else:
+                fixed.append(word)
+        return ' '.join(fixed) if changed else None
+
+
+def _acceptable(word, candidate):
+    """Guards that keep a correction from destroying an identifier.
+
+    DIGITS MUST MATCH EXACTLY. CSF16 to CSF17 is one edit and a catastrophe: it
+    is a different bay, a different machine, a different cartridge. Any term
+    carrying digits is treated as an identifier, and an identifier is either
+    right or not correctable.
+
+    THE FIRST CHARACTER MUST MATCH. Typing errors land in the middle and at the
+    end far more often than on the first key, so requiring it costs almost no
+    recall and removes most of the nonsense.
+    """
+    if _digits(word) != _digits(candidate):
+        return False
+    return bool(word) and bool(candidate) and word[0] == candidate[0]