Repository navigation
fix(python-sdk): validate recall query and max_distance (#1248) - #1254
Conversation
ducnmm
left a comment
There was a problem hiding this comment.
[P2] Include the Python release update with this SDK change.
The branch still carries Python 0.1.11, while current dev and main both carry 0.1.12 and that version is already on PyPI. This PR does not prepare a new release. After rebasing, keeping 0.1.12 makes release-python-sdk.yml skip stable publication, so users cannot receive this fix. Please prepare the next patch (currently 0.1.13), synchronize pyproject.toml, memwal/__init__.py, the package and docs changelogs, and the verifier pin. Re-evaluate the version if another unreleased patch lands first; use the manual release path.
Validation: 268 non-integration Python tests passed; Ruff clean. No new runtime regression found.
0468f41 to
5713271
Compare
|
@ducnmm rebased on dev (one conflict in |
ducnmm
left a comment
There was a problem hiding this comment.
[P2] Retarget the release to 0.2.1 after rebasing. dev and main now carry 0.2.0 (packages/python-sdk-memwal/pyproject.toml:7, memwal/__init__.py:131), and 0.2.0 is already the version on PyPI. This branch adds 0.1.13 (packages/python-sdk-memwal/CHANGELOG.md:3, docs/python-sdk/changelog.mdx:39) and calls it the latest release (docs/python-sdk/changelog.mdx:32), which is now below the current release; the PR is also CONFLICTING against dev. Keeping 0.1.13 would downgrade the SDK, and since 0.1.13 was never published, pip install memwal would continue to resolve 0.2.0, so users would not receive this fix. Please rebase and move the entry under a 0.2.1 section, updating pyproject.toml:7, memwal/__init__.py:131, and scripts/verify-manual-sdk-release.mjs:14 to 0.2.1.
|
@ducnmm rebased onto |
5713271 to
22556f9
Compare
22556f9 to
54be5f3
Compare
54be5f3 to
0ba6bc0
Compare
ducnmm
left a comment
There was a problem hiding this comment.
Please address the findings below before merging.
0ba6bc0 to
51c8b3a
Compare
|
@ducnmm Rebased onto
|
ducnmm
left a comment
There was a problem hiding this comment.
Re-reviewed the current head. Recall query/max-distance validation is consistent across the real client and mock, and the fix targets the unreleased Python 0.2.2 patch. Seventeen focused local tests passed. Current CI checks are successful or skipped.
recall() signed an empty query that MemWalMock.recall() already rejects, and a NaN or negative max_distance dropped every hit because the filter keeps distance < max_distance. Reject a blank query with the mock's message, and reject a non-finite or negative max_distance in both the client and the mock, matching the TypeScript guards from Fixes #1248
51c8b3a to
a932327
Compare
Summary
This is the Python side of #1215/#1228 and #1212/#1227, which were fixed in TypeScript only.
MemWal.recall("")signed and sent an empty query, even thoughMemWalMock.recall("")raisesValueError: query cannot be empty.max_distance=float("nan")or a negative value dropped every hit, because the filter keepsdistance < max_distance. The empty result looks the same as an empty namespace. The mock had the same filter and the same problem.Changes
recall()rejects a blank query with the mock's message (query cannot be empty) before signing. This covers both the positional andRecallParamscall styles.memwal.utils.validate_max_distance, used by bothMemWal.recallandMemWalMock.recall. It rejects NaN, ±inf, negative,booland non-numeric values withValueError("max_distance must be a non-negative finite number"), the same rule as the TypeScriptmaxDistanceguard.Tests
tests/test_recall_validation.py: blank queries and badmax_distancevalues are rejected before signing in the client and in the mock. Valid thresholds still filter.ruff check memwal/ tests/andpytest tests/ -m "not integration"pass.Estimate: 2 SP (incl. review)
Local results: ruff clean; pytest -m "not integration": 268 passed (new test failed 15/17 before the fix); mypy unchanged vs dev.
Fixes #1248