Skip to content

Commit b4bd4df

Browse files
galshubeliclaude
andcommitted
fix(usage): correct success semantics, env-agnostic test, doc wording
Address the latest review pass: - routes/graphs: success = is_valid AND no error. error_message-None alone counted off-topic / not-SQL-translatable results (is_valid=False, no error) as successes, inflating success_count. - tests: assert against usage_tracking.ORGANIZATIONS_GRAPH instead of the hardcoded "Organizations" so the suite passes when the env var is set. - usage_tracking: reword the task_sink docstring — it no longer ships in the SDK, so drop the QueryWeaver.close() reference for a generic description. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent e47e580 commit b4bd4df

3 files changed

Lines changed: 8 additions & 3 deletions

File tree

api/routes/graphs.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,12 @@ async def _serialize_pipeline(gen, *, user_id, namespaced):
5555
break
5656
yield json.dumps(event) + MESSAGE_DELIMITER
5757
if final is not None and not final.requires_confirmation:
58+
# "Success" = a valid query that ran without error. error_message is
59+
# None alone isn't enough: off-topic / not-SQL-translatable results
60+
# carry is_valid=False with no error, and must not inflate success_count.
5861
record_query_usage_background(
59-
user_id, namespaced, success=final.error_message is None
62+
user_id, namespaced,
63+
success=final.is_valid and final.error_message is None,
6064
)
6165

6266

api/routes/usage_tracking.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,8 @@ def record_query_usage_background(
120120
success: Whether SQL execution succeeded (no execution error).
121121
db: Optional FalkorDB handle; resolves to the server singleton when None.
122122
task_sink: Optional set the scheduled task is added to (and auto-removed
123-
from on completion) so SDK ``QueryWeaver.close()`` can await it.
123+
from on completion) so callers can await any in-flight tracking
124+
writes before shutdown.
124125
"""
125126
email = _decode_email(user_id)
126127
if email is None:

tests/test_usage_tracking.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ async def test_records_successful_query_event(self):
6868
)
6969
await _drain(sink)
7070

71-
db.select_graph.assert_called_once_with("Organizations")
71+
db.select_graph.assert_called_once_with(usage_tracking.ORGANIZATIONS_GRAPH)
7272
graph.query.assert_awaited_once()
7373
cypher, params = graph.query.await_args.args
7474
assert "MATCH (u:User {email: $email})" in cypher

0 commit comments

Comments
 (0)