Fix multiword terms created from the reading pane silently failing to match (context-sensitive parsers)

[iinfonacho-art]

Fix: multiword terms created from the reading pane can silently fail to match (context-sensitive parsers)

Base: 3.10.1 (09bb17b)

Summary

Creating a multiword term by selecting text in the reading pane (the
documented, supported way to do it) can silently produce a term that
never highlights again when reading — it just falls back to the
individual single-word tokens, with no error shown anywhere.

This reproduces reliably for Japanese (MeCab), and will affect any
other parser whose tokenization is context-sensitive.

Repro

1. Language: Japanese. Text containing: ...それはそれで問題になる...
2. In the reading pane, drag-select the 4 tokens それ | は | それ | で
   and create a new term from them.
3. Reload/reopen the text.
4. Expected: the 4 tokens render as one highlighted multiword term.
   Actual: they render as 4 separate tokens again, as if the term
   didn't exist.

Inspecting the DB confirms the term was saved with the wrong
token count:

WoText = 'それ\u200bは\u200bそれで'   -- 3 tokens (それ / は / それで)
WoTokenCount = 3

instead of the correct:

WoText = 'それ\u200bは\u200bそれ\u200bで'   -- 4 tokens
WoTokenCount = 4

Root cause

MeCab's segmentation is context-sensitive. Parsed on its own, それはそれで
comes back as 3 tokens, because MeCab treats trailing それで as a single
conjunction (接続詞, "so"/"therefore"):

$ echo "それはそれで" | mecab
それ    名詞,代名詞,一般,...
は      助詞,係助詞,...
それで  接続詞,...

Parsed as part of its actual sentence, MeCab correctly keeps で as its
own particle attaching to 問題になる:

$ echo "...それはそれで問題になるようになった。" | mecab
...
それ    名詞,代名詞,一般,...
は      助詞,係助詞,...
それ    名詞,代名詞,一般,...
で      助詞,格助詞,...
問題    ...

Both segmentations are individually "correct" — it's genuine
disambiguation that depends on what follows.

Tracing this through the code:

• The reading pane builds the text for a new multiword term by
  reading .text() off each selected (already-tokenized, on-screen)
  word span, then joining them with '', discarding the token
  boundaries the page's own parse had already established:

  lute/static/js/lute.js, show_multiword_term_edit_form():

  const textparts = selected.toArray().map((el) => $(el).text());
  const text = textparts.join('').trim();   // <-- boundaries lost here

• That flat string is sent to GET/POST /read/termform/<langid>/<text>
  (lute/read/routes.py), which calls Repository.find_or_new(langid, usetext).

• find_or_new (lute/term/model.py) builds a "spec" term via
  DBTerm(lang, text), which goes through Term.text setter →
  Term._parse_string_add_zws() (lute/models/term.py). That method
  always re-parses the string from scratch via lang.get_parsed_tokens(t),
  regardless of any structure already present — it even explicitly strips
  any pre-existing zero-width-space (zws) separators before doing so
  (t = t.replace(zws, "")).

So even though the reading pane had the correct, in-context token
boundaries at the moment of selection, they never reach the backend,
and the backend actively discards them even if they did.

This isn't a new discovery for the codebase — find_or_new's own
docstring already flags the general problem:

"the parsing here is done without a fuller context, which in some
language parsers can result in different results... So what does
this mean? It means that any context-less searches for terms that
have ambiguous parsing results will, themselves, also be ambiguous.
This impacts csv imports and term form usage."

and there's an existing characterization test for it,
test_find_or_new_ambiguous_japanese_terms, using the same style of
example (集めれ → 集め/れ). There's also Term.create_term_no_parsing(),
added specifically to let other parts of the code (page rendering,
in lute/read/render/calculate_textitems.py) skip this re-parsing step
when they already have trustworthy, in-context tokens. This PR extends
that same idea to the one remaining place that needed it: multiword term
creation from the reading pane.

The fix

Two small, surgical changes, no new dependencies or config:

1. lute/static/js/lute.js

show_multiword_term_edit_form() now joins the selected token texts
with a zero-width space (\u200B) — the same separator Lute already
uses internally to mark token boundaries — instead of ''. This
preserves the exact in-context tokenization at the point of creation.

2. lute/models/term.py

Term._parse_string_add_zws() now checks whether the incoming text
already contains zws. If it does, it's treated as pre-tokenized and
used as-is (just trimming/tidying), instead of being re-parsed. If it
doesn't contain zws, behaviour is completely unchanged — plain text
(CSV import, manually typed text in term forms/search) still gets
parsed fresh, exactly as before.

This keeps the fix scoped to exactly the case that has good
information available (reading-pane selections), without changing
behaviour for cases that don't (CSV import, freeform typed text),
which remain as ambiguous as before — that's a separate, larger
problem (also called out in find_or_new's docstring) that would
need actual sentence context to be passed around, not just addressed
by trusting zws.

Tests

Added:

• tests/orm/test_Term.py
  • test_new_term_with_pretokenized_zws_text_is_not_reparsed — a
    zws-joined それ/は/それ/で is kept as 4 tokens, not collapsed to 3.
  • test_new_term_without_zws_is_parsed_from_scratch_as_before —
    plain (non pre-tokenized) text keeps the old (ambiguous, 3-token)
    behaviour, unchanged.
• tests/unit/term/test_Repository.py
  • test_find_or_new_with_pretokenized_text_finds_in_context_term —
    find_or_new given pre-tokenized text finds the existing
    in-context term instead of missing it and creating a mismatched one.
  • test_find_or_new_with_pretokenized_text_creates_correct_new_term —
    same, for the not-yet-existing case.

All 4 new tests fail on 3.10.1 (reproducing the bug) and pass with
this change. Full existing suite (tests/unit + tests/orm, 424
tests, plus the 56 in the two files touched) still passes unchanged —
this includes the pre-existing test_find_or_new_ambiguous_japanese_terms
characterization test, which is unaffected since its input never
contains zws.

$ python3 -m pytest tests/unit tests/orm -q
424 passed

Out of scope

• CSV import and the plain "type text into a form" flow have no
  reliable source of in-context tokenization to draw on, so they're
  intentionally left as-is (still subject to the same MeCab ambiguity,
  same as today).
• No attempt is made here to retroactively fix multiword terms that
  were already mis-tokenized by this bug before upgrading. That's a
  data migration, not a code fix; the DB fields involved are
  WoText / WoTextLC / WoTokenCount on the words table, in case
  a follow-up cleanup script is wanted.

Checklist

• Reproduced the bug against a fresh 3.10.1 checkout.
• Root-caused to specific lines in lute.js and lute/models/term.py.
• Minimal fix, no behaviour change for CSV import / typed text.
• New unit tests added; confirmed they fail without the fix and
  pass with it.
• Full tests/unit + tests/orm suite passes (424 passed).
• black clean on all touched files.
• Manual smoke test in a running instance (drag-select a multiword
  term in Japanese, confirm it highlights correctly after reload) —
  worth doing once more before merge, since the JS side isn't
  covered by an automated test.

[jzohrab]

Thanks for the PR, appreciated.

This is odd as I recall adding code to not reparse terms that are created via the reading pane ... this was a big change, in #347 :-) That PR changed the code to create "status 0 (unknown)" terms during the parse, so things would be handled correctly and avoid the problem mentioned in this PR. I thought I had it all covered! But bugs happen, so thanks for checking!

[jzohrab]

My prior comment was actually not useful here -- that old PR was to prevent re-parsing of terms from the reading window, but in this case, the user is creating a new multi-word term from the terms in the reading pane.

I still thought that the code handled this correctly! If it didn't, that's a big error, and it's surprising it went so long undetected. Thanks, checking :-)

[jzohrab]

Some small changes, and a bigger one with the javascript change and the '*' url hack. Cheers!

[jzohrab]

This is concerning, the code was changed to use the "dicttype", and not rely on special tokens within the URL. Changed in #331. This code as given here should never work.

[jzohrab]

Well, not "should never work", "would never work".

[jzohrab]

This isn't a "term_case" test :-)

[jzohrab]

Not a term_case test.

"_as_before" should be removed, this test will be read as a standalone thing by future maintainers, so references to history gets a bit confusing (unless this is a characterization test, which is a different kind of thing)