fix(telemetry): the user-scoped rule_usage export read _rule_ids before it existed (#3315)
CI & Build / Python lint (push) Successful in 5s
CI & Build / Plugin hooks (push) Successful in 10s
CI & Build / integration (push) Successful in 29s
CI & Build / TypeScript typecheck (push) Successful in 33s
CI & Build / Python tests (push) Successful in 1m6s
CI & Build / Build & push image (push) Successful in 25s

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) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TcCs1CcQ1ormdnzSshKqvN
This commit is contained in:
2026-09-02 16:56:54 -04:00
co-authored by Claude Opus 5
parent 8826be7a91
commit 111eef7e30
+8 -8
View File
@@ -767,14 +767,6 @@ async def export_user_backup(user_id: int) -> dict:
usage_events = (await session.execute( usage_events = (await session.execute(
select(NoteUsageEvent).where(NoteUsageEvent.note_id.in_(note_ids)) select(NoteUsageEvent).where(NoteUsageEvent.note_id.in_(note_ids))
)).scalars().all() if note_ids else [] )).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( repo_bindings = (await session.execute(
select(RepoBinding).where(RepoBinding.user_id == user_id) select(RepoBinding).where(RepoBinding.user_id == user_id)
)).scalars().all() )).scalars().all()
@@ -826,6 +818,14 @@ async def export_user_backup(user_id: int) -> dict:
select(RuleVersion).where(RuleVersion.rule_id.in_(_rule_ids)) select(RuleVersion).where(RuleVersion.rule_id.in_(_rule_ids))
.order_by(RuleVersion.rule_id, RuleVersion.id) .order_by(RuleVersion.rule_id, RuleVersion.id)
)).scalars().all() if _rule_ids else [] )).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( rule_relations = (await session.execute(
select(RuleRelation).where( select(RuleRelation).where(
RuleRelation.from_rule_id.in_(_rule_ids), RuleRelation.from_rule_id.in_(_rule_ids),