feat: v2.31.0 — replace speedict with sqlite3 and require Python 3.13+ - #61
Merged
drawal1 merged 2 commits intoAug 8, 2026
Merged
Conversation
…+ (fix-lvl) Drop the abandoned speedict/RocksDB dependency in favor of stdlib sqlite3 (WAL, JSON values, float32 BLOB utterance cache). Unblocks Python 3.13 installs, removes pickle-on-disk from first-party stores, and fixes process-exclusive LOCK failures under concurrent writers. requires-python is now >=3.13,<3.15; pre-existing .rdb / cache.db RocksDB dirs are abandoned. Co-authored-by: Cursor <cursoragent@cursor.com>
Reviewer's GuideReplaces the abandoned speedict/RocksDB dependency with a stdlib sqlite3-backed KVStore (including a specialized UtteranceCacheStore) across caches and conversation persistence, raises the Python version requirement to 3.13–3.14, and adds concurrency and behavior tests to validate the migration and JSON-only storage. Sequence diagram for utterance caching and matching with UtteranceCacheStoresequenceDiagram
actor User
participant IntentDetection as intent_detection
participant CacheMatching as cache_matching
participant Store as UtteranceCacheStore
User->>IntentDetection: store_utterance_cache(cache_path, utterance, label, model_pipeline)
IntentDetection->>CacheMatching: store_utterance_cache(cache_path, utterance, label, model_pipeline)
CacheMatching->>Store: UtteranceCacheStore(cache_path)
CacheMatching->>CacheMatching: get_embedding(utterance, model_pipeline)
CacheMatching->>Store: get(utterance_hash)
Store-->>CacheMatching: existing entry or None
CacheMatching->>Store: upsert(utterance_hash, utterance, command_mapping, embedding)
CacheMatching->>Store: close()
User->>IntentDetection: cache_match(cache_path, utterance, model_pipeline)
IntentDetection->>CacheMatching: cache_match(cache_path, utterance, model_pipeline)
CacheMatching->>Store: UtteranceCacheStore(cache_path)
CacheMatching->>CacheMatching: get_embedding(utterance, model_pipeline)
CacheMatching->>Store: iter_entries()
Store-->>CacheMatching: (hash_key, entry) list
CacheMatching->>CacheMatching: cosine_similarity(query_embedding, cached_embedding)
CacheMatching-->>IntentDetection: best label / None
CacheMatching->>Store: close()
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 security issues, 3 other issues, and left some high level feedback:
Security issues:
- Avoiding SQL string concatenation: untrusted input concatenated with raw SQL query can result in SQL Injection. In order to execute raw query safely, prepared statement should be used. SQLAlchemy provides TextualSQL to easily used prepared statement with named parameters. For complex SQL composition, use SQL Expression Language or Schema Definition Language. In most cases, SQLAlchemy ORM will be a better option. (link)
- Avoiding SQL string concatenation: untrusted input concatenated with raw SQL query can result in SQL Injection. In order to execute raw query safely, prepared statement should be used. SQLAlchemy provides TextualSQL to easily used prepared statement with named parameters. For complex SQL composition, use SQL Expression Language or Schema Definition Language. In most cases, SQLAlchemy ORM will be a better option. (link)
General comments:
- Now that KVStore and UtteranceCacheStore implement context managers, consider using
within the call sites (e.g., cache_matching, intent_detection helpers) instead of manualclose()calls to make lifecycle management more robust and less error-prone. - KVStore stores values as JSON
TEXTin a singlevcolumn; if you anticipate large or complex values (e.g., big payloads), you may want to revisit this schema (e.g.,BLOBor additional typed columns) to avoid unnecessary UTF-8 encode/decode overhead and potential size-related performance issues.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Now that KVStore and UtteranceCacheStore implement context managers, consider using `with` in the call sites (e.g., cache_matching, intent_detection helpers) instead of manual `close()` calls to make lifecycle management more robust and less error-prone.
- KVStore stores values as JSON `TEXT` in a single `v` column; if you anticipate large or complex values (e.g., big payloads), you may want to revisit this schema (e.g., `BLOB` or additional typed columns) to avoid unnecessary UTF-8 encode/decode overhead and potential size-related performance issues.
## Individual Comments
### Comment 1
<location path="fastworkflow/kvstore.py" line_range="168-174" />
<code_context>
+ )
+ self._conn.commit()
+
+ def iter_entries(self) -> Iterator[tuple[str, dict[str, Any]]]:
+ rows = self._conn.execute(
+ "SELECT k, meta, vec FROM utterance_cache"
+ ).fetchall()
+ for key, meta_json, vec in rows:
+ meta = json.loads(meta_json)
+ yield key, {
+ "utterance": meta.get("utterance", ""),
+ "command_mapping": meta.get("command_mapping", {}),
</code_context>
<issue_to_address>
**suggestion (performance):** UtteranceCacheStore.iter_entries uses fetchall(), which may not scale for large caches.
iter_entries() loads the entire `utterance_cache` table into memory via `fetchall()`, which can cause high memory usage and GC overhead for large caches, especially when `cache_match` builds `list(db.iter_entries())`. Consider iterating directly over the cursor (e.g. `for key, meta_json, vec in self._conn.execute(...):`) and having `cache_match` consume that iterator instead of materialising a list when only the best match is required.
</issue_to_address>
### Comment 2
<location path="tests/test_kvstore.py" line_range="115-124" />
<code_context>
+ raise SystemExit(1) from exc
+
+
+def test_kvstore_four_process_concurrency(tmp_path: Path):
+ path = str(tmp_path / "concurrent.sqlite3")
+ n_workers = 4
+ n_ops = 50
+ result_paths = [str(tmp_path / f"result_{i}.txt") for i in range(n_workers)]
+ procs = [
+ multiprocessing.Process(
+ target=_mp_writer, args=(path, i, n_ops, result_paths[i])
+ )
+ for i in range(n_workers)
+ ]
+ for p in procs:
+ p.start()
+ for p in procs:
+ p.join(timeout=60)
+ assert p.exitcode == 0, f"worker exit {p.exitcode}"
+
+ for rp in result_paths:
+ assert Path(rp).read_text(encoding="utf-8") == "ok"
+
+ with KVStore(path) as db:
+ keys = list(db.keys())
+ assert len(keys) == n_workers * n_ops
</code_context>
<issue_to_address>
**suggestion (testing):** Concurrency test should also validate stored values, not just key count
This test only checks the total key count. To catch silent data corruption under concurrent writes, it should also validate that each key’s stored `(worker, i)` payload is correct (e.g., iterate `db.keys()` and assert `db[key]['worker']` / `db[key]['i']` follow the expected pattern). That would more robustly exercise WAL + `busy_timeout` under contention.
</issue_to_address>
### Comment 3
<location path="tests/test_kvstore.py" line_range="64-73" />
<code_context>
+ assert b"DefinitelyNotPickle" in raw # plain JSON text
+
+
+def test_utterance_cache_float32_blob_round_trip(tmp_path: Path):
+ path = str(tmp_path / "utt.sqlite3")
+ vec = np.arange(8, dtype=np.float32)
+ with UtteranceCacheStore(path) as store:
+ store.upsert(
+ "123",
+ utterance="hello",
+ command_mapping={"cmd": {"frequency": 1, "feedback_date": "t"}},
+ embedding=vec,
+ )
+ entry = store.get("123")
+ assert entry is not None
+ assert entry["utterance"] == "hello"
+ assert entry["command_mapping"]["cmd"]["frequency"] == 1
+ np.testing.assert_array_equal(entry["embedding"], vec)
+
+ entries = list(store.iter_entries())
+ assert len(entries) == 1
+ assert entries[0][0] == "123"
+
+
</code_context>
<issue_to_address>
**suggestion (testing):** Add tests for UtteranceCacheStore handling of None/empty embeddings
The current round-trip test only covers non-empty float32 vectors. To exercise the BLOB packing/unpacking edge cases in `_pack_vec/_unpack_vec`, please add tests that upsert and then read back entries with `embedding=None` and `embedding=np.array([], dtype=np.float32)`. These should confirm that both paths yield `embedding is None` on read and that `cache_match` safely skips such entries.
Suggested implementation:
```python
from fastworkflow.kvstore import KVStore, UtteranceCacheStore
def test_utterance_cache_none_embedding_round_trip(tmp_path: Path):
path = str(tmp_path / "utt_none.sqlite3")
with UtteranceCacheStore(path) as store:
store.upsert(
"none-emb",
utterance="hello-none",
command_mapping={"cmd": {"frequency": 1, "feedback_date": "t"}},
embedding=None,
)
entry = store.get("none-emb")
assert entry is not None
assert entry["utterance"] == "hello-none"
assert entry["command_mapping"]["cmd"]["frequency"] == 1
assert entry["embedding"] is None
entries = list(store.iter_entries())
assert len(entries) == 1
assert entries[0][0] == "none-emb"
assert entries[0][1]["embedding"] is None
def test_utterance_cache_empty_embedding_round_trip(tmp_path: Path):
path = str(tmp_path / "utt_empty.sqlite3")
empty_vec = np.array([], dtype=np.float32)
with UtteranceCacheStore(path) as store:
store.upsert(
"empty-emb",
utterance="hello-empty",
command_mapping={"cmd": {"frequency": 1, "feedback_date": "t"}},
embedding=empty_vec,
)
entry = store.get("empty-emb")
assert entry is not None
assert entry["utterance"] == "hello-empty"
assert entry["command_mapping"]["cmd"]["frequency"] == 1
# Empty vectors should round-trip to a None embedding, not an empty ndarray
assert entry["embedding"] is None
entries = list(store.iter_entries())
assert len(entries) == 1
assert entries[0][0] == "empty-emb"
assert entries[0][1]["embedding"] is None
```
To fully cover the `cache_match` behavior, you should also:
1. Import the `cache_match` helper from `fastworkflow.kvstore` (or wherever it is defined).
2. Add tests that:
- Create a `UtteranceCacheStore` containing entries with `embedding=None` and empty embeddings.
- Call `cache_match(...)` with a non-empty query embedding.
- Assert that:
* No matches are returned for the entries with `embedding is None`, or
* At minimum, `cache_match` does not raise and does not attempt to compute similarity against the `None`/empty embeddings.
You will need to adapt the test according to the actual `cache_match` signature and return type (e.g., list of matches, best match, etc.).
</issue_to_address>
### Comment 4
<location path="fastworkflow/kvstore.py" line_range="36" />
<code_context>
self._conn.execute(f"PRAGMA busy_timeout={int(timeout * 1000)}")
</code_context>
<issue_to_address>
**security (python.sqlalchemy.security.sqlalchemy-execute-raw-query):** Avoiding SQL string concatenation: untrusted input concatenated with raw SQL query can result in SQL Injection. In order to execute raw query safely, prepared statement should be used. SQLAlchemy provides TextualSQL to easily used prepared statement with named parameters. For complex SQL composition, use SQL Expression Language or Schema Definition Language. In most cases, SQLAlchemy ORM will be a better option.
*Source: opengrep*
</issue_to_address>
### Comment 5
<location path="fastworkflow/kvstore.py" line_range="107" />
<code_context>
self._conn.execute(f"PRAGMA busy_timeout={int(timeout * 1000)}")
</code_context>
<issue_to_address>
**security (python.sqlalchemy.security.sqlalchemy-execute-raw-query):** Avoiding SQL string concatenation: untrusted input concatenated with raw SQL query can result in SQL Injection. In order to execute raw query safely, prepared statement should be used. SQLAlchemy provides TextualSQL to easily used prepared statement with named parameters. For complex SQL composition, use SQL Expression Language or Schema Definition Language. In most cases, SQLAlchemy ORM will be a better option.
*Source: opengrep*
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Stream utterance_cache rows instead of fetchall, prefer context managers at call sites, rely on sqlite3.connect(timeout=) instead of interpolating PRAGMA busy_timeout, and harden concurrency/empty-embedding tests. Co-authored-by: Cursor <cursoragent@cursor.com>
5 tasks
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
CVE automation follow-up (post-merge)
Multi-scanner pass completed against the merged v2.31.0 tree (092ea6a).
| Tool | Result |
|---|---|
| Syft / Trivy / Grype / osv-scanner | OK — 1 Python advisory (diskcache CVE-2025-69872), OpenVEX suppresses it |
| Dockle / Dive | OK on proxy image python:3.13-slim-bookworm (no first-party Dockerfile) |
| Snyk / Docker Scout | Skipped (no SNYK_TOKEN / Docker Hub login) |
Easy pyproject.toml remediations: none (diskcache still has no PyPI fix).
Human-review HTML: all current findings recommended IGNORE with rationale (diskcache VEX’d; proxy-image OS/CIS findings out of product scope).
Follow-up PR with refreshed reports: #62
Sent by Cursor Automation: Untitled
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
speedict/RocksDB with stdlibsqlite3(KVStore+ float32 BLOBUtteranceCacheStore) across enablecache, NLU cache, utterance matching, and conversation storerequires-pythonto>=3.13,<3.15(unblocks 3.13 installs; no sdist/wheel existed for speedict on 3.13+).rdb/ RocksDBcache.dbdirs (no data migration); coerce classifierndarraylabels before JSON storefix-lvlTest plan
test_kvstore, enablecache, conversation concurrency, cache matching, memory bounds, fastapi service paths)PYTEST_EXIT=0, ~48 min)speedict→rocksdictshims after upgrade.rdb/___convo_info/cache.dbRocksDB directories when readyMade with Cursor
Summary by Sourcery
Replace RocksDB/speedict-based persistence and caches with a new SQLite-backed key-value store and float32 BLOB utterance cache, and raise the minimum supported Python version to 3.13.
New Features:
Enhancements:
Build:
Documentation:
Tests: