From 111eef7e30d51c27bd7ebf04fff796c5a63f939b Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Wed, 2 Sep 2026 16:56:54 -0400 Subject: [PATCH] fix(telemetry): the user-scoped rule_usage export read _rule_ids before it existed (#3315) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ruff F821, twice, on the same two lines. The query was placed next to its note-usage counterpart — which reads `note_ids`, defined much earlier — while `_rule_ids` is not built until forty lines further down, beside the rules themselves. Moved to sit directly after the `rule_versions` query, which is the other consumer of that variable and the block whose scoping argument this one restates. Worth noting what did NOT catch this. The integration round-trip passed on the same commit: it drives `restore_full_backup` against a hand-built payload, so it exercises the import side and the full export, and never calls `export_user_backup` at all. A per-user export of any account owning a rule would have raised NameError at runtime. The lint lane found it because a static check does not need the path to be reachable by a test. The comment moved with it and got sharper, since the hazard is that the plausible column is the wrong one: `user_id` on a usage row is whoever the arm fired FOR, not who owns the rule, so scoping a per-user export by it would carry this user's surfacings of someone else's rule and drop the ones fired for someone else on theirs. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01TcCs1CcQ1ormdnzSshKqvN --- src/scribe/services/backup.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/scribe/services/backup.py b/src/scribe/services/backup.py index 6491975..50e9f82 100644 --- a/src/scribe/services/backup.py +++ b/src/scribe/services/backup.py @@ -767,14 +767,6 @@ async def export_user_backup(user_id: int) -> dict: usage_events = (await session.execute( select(NoteUsageEvent).where(NoteUsageEvent.note_id.in_(note_ids)) )).scalars().all() if note_ids else [] - # Scoped through the RULE, not the event's user_id — the same call - # rule_versions makes one block up. user_id here is whoever the arm - # fired for, so filtering on it would carry this user's surfacings of - # someone ELSE's rule and drop the ones fired for someone else on - # theirs: the opposite of a per-user export. - rule_usage_events = (await session.execute( - select(RuleUsageEvent).where(RuleUsageEvent.rule_id.in_(_rule_ids)) - )).scalars().all() if _rule_ids else [] repo_bindings = (await session.execute( select(RepoBinding).where(RepoBinding.user_id == user_id) )).scalars().all() @@ -826,6 +818,14 @@ async def export_user_backup(user_id: int) -> dict: select(RuleVersion).where(RuleVersion.rule_id.in_(_rule_ids)) .order_by(RuleVersion.rule_id, RuleVersion.id) )).scalars().all() if _rule_ids else [] + # Scoped through the RULE for the same reason the versions above are, + # and it is worth restating because the column that looks right is + # wrong: `user_id` here is whoever the arm fired FOR, not who owns the + # rule. Filtering on it would carry this user's surfacings of someone + # ELSE's rule and drop the ones fired for someone else on theirs. + rule_usage_events = (await session.execute( + select(RuleUsageEvent).where(RuleUsageEvent.rule_id.in_(_rule_ids)) + )).scalars().all() if _rule_ids else [] rule_relations = (await session.execute( select(RuleRelation).where( RuleRelation.from_rule_id.in_(_rule_ids),