Skip to content

fix(search): keep mypy clean under sentence-transformers 6.0.0 - #95

Open
hey-august wants to merge 1 commit into
mainfrom
august-20260818-sentence-transformers-6-typecheck
Open

fix(search): keep mypy clean under sentence-transformers 6.0.0#95
hey-august wants to merge 1 commit into
mainfrom
august-20260818-sentence-transformers-6-typecheck

Conversation

@hey-august

@hey-august hey-august commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

What this changes

The problem

sentence-transformers 6.0.0 was integrated on 2026-08-18 and broke the [TYPECHECK] check on every open PR in this repository.

The pin is unbounded (sentence-transformers>=2.2.0), so CI picked up the new major version. mypy and torch are unchanged. Nothing in the tree moved, signalwire/search/ last changed on Aug 13 and these same branches passed Test on Aug 17.

All four errors are in signalwire/search/:

  • Two errors come from model.model_name = model_name, where mypy now resolves the model to a torch nn.Module subclass whose __setattr__ accepts only Tensor and Module.
  • An additional two errors come from if not SentenceTransformer, which reads as a truthy-function error because the TYPE_CHECKING import binds the real class and is never None.

The solution, in this PR

The fix annotates those two locals as model: Any, matching the dict[str, Any] declarations sitting beside them, and makes the availability guards explicit is None checks. It adapts to 6.x rather than capping the dependency, since a cap would transitively hold back transformers.

Two tidier-looking alternatives fail. setattr(model, "model_name", ...) trips ruff B010 and fails the LINT gate. Moving availability onto a separate boolean breaks test_load_model_no_library, because the tests simulate the missing dependency by patching the module-level name to None, so the guard has to keep reading that name. The model_name attribute itself also has to stay: it looks like a dead write, but query_processor.py:243 reads it back through getattr for set_global_model.

Verified mypy-clean against both sentence-transformers 6.0.0 and 5.6.0, ruff check and format clean, and the full unit suite green (5772 passed, 100 skipped).

Checklist

  • bash scripts/run-ci.sh passes locally
  • New tests assert on content (not just "does not raise" / "is not None")
  • New test functions are type-annotated (mypy covers tests/)

run-ci.sh: 31 gates pass locally, including TYPECHECK (the gate this PR fixes), TEST, LINT, FMT and SIGNATURES.

But five fail for reasons unrelated to this change:

  • SEMVER-DIFF, SURFACE-NATIVE and ROUTE-COLLISION expect other repos, porting-sdk and the SDK, at paths that do not match my filesystem organization.
  • DOC-AUDIT cascades from SURFACE-NATIVE
  • DOC-SURFACE reports an identical 64.3% on origin/main with and without this commit.

Leaving the box un-checked.

No new tests. This is a typing-only change with no behavioral surface of its own, and the guards it rewrites are already covered: test_load_model_no_library exercises the SentenceTransformer is None path by patching the module-level name to None. That test is also what constrained the fix, since it rules out moving the availability check onto a separate flag.

Does this change public API?

  • No
  • Yes — naming it here so a maintainer can land the matching
    infrastructure change:

Changing something that goes on the wire?

No. The change is confined to signalwire/signalwire/search/, which loads local embedding models. Nothing here is emitted, no enum values or parameter names move, and no generated types are touched.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant