tux icon indicating copy to clipboard operation
tux copied to clipboard

refactor(treewide): reorder statements to improve speed

Open amadaluzia opened this issue 5 months ago • 5 comments

Description

This refactors the tree to optimise the cog loader, the avatar command and the xkcd command with reordered if statements, and storing variables to remove unnecessary function calls. I think this is clean enough at the moment for me to write this as a PR. There will of course still be some unoptimal things I have missed but I cannot seem to find any that wouldn't conflict with #853.

Guidelines

  • [x] My code follows the style guidelines of this project (formatted with Ruff)

  • [x] I have performed a self-review of my own code

  • [ ] I have commented my code, particularly in hard-to-understand areas

  • [ ] I have made corresponding changes to the documentation if needed

  • [x] My changes generate no new warnings

  • [x] I have tested this change

  • [ ] Any dependent changes have been merged and published in downstream modules

  • [ ] I have added all appropriate labels to this PR

  • [ ] I have followed all of these guidelines.

How Has This Been Tested? (if applicable)

I started Tux and ran a few commands.

Screenshots (if applicable)

{DA6308F5-4136-4C22-A265-C737785F4DC8}

Additional Information

Please add any other information that is important to this PR.

Summary by Sourcery

Refactor sentry span tagging and command handlers to reduce redundant calls and simplify control flows

Enhancements:

  • Cache sentry initialization status in a module-level variable to eliminate repeated is_initialized calls in cog_loader
  • Reorder and combine SENTRY_INITIALIZED checks around span tagging to streamline cog loading observability
  • Simplify avatar command logic by consolidating interaction response paths and removing duplicate branches
  • Refactor xkcd command to use an early return for missing comic_id and reorder specific comic handling

amadaluzia avatar Jul 16 '25 21:07 amadaluzia

Reviewer's Guide

This PR optimizes the cog loader and command handlers by caching Sentry initialization to eliminate redundant calls, and by streamlining conditional logic in the avatar and xkcd commands to simplify control flow and improve performance.

Sequence diagram for optimized Sentry initialization in cog loader

sequenceDiagram
    participant CogLoader
    participant SENTRY_INITIALIZED
    participant sentry_sdk
    participant current_span
    CogLoader->>SENTRY_INITIALIZED: Check if Sentry is initialized (cached)
    alt SENTRY_INITIALIZED is True
        CogLoader->>sentry_sdk: get_current_span()
        alt current_span exists
            CogLoader->>current_span: set_tag/set_data
        end
    end

Sequence diagram for refactored avatar command handler

sequenceDiagram
    participant User as actor
    participant AvatarCommandHandler
    participant discord.Interaction
    User->>AvatarCommandHandler: send_avatar(source, member)
    alt source is discord.Interaction and member exists
        AvatarCommandHandler->>discord.Interaction: response.send_message(files=files)
    else member has no avatar
        AvatarCommandHandler->>discord.Interaction: response.send_message(content=message, ephemeral=True, delete_after=30)
    else source is commands.Context
        AvatarCommandHandler->>AvatarCommandHandler: member = MemberConverter().convert(source, author.id)
    end

Sequence diagram for refactored xkcd command handler

sequenceDiagram
    participant User as actor
    participant XKCDCommandHandler
    participant commands.Context
    User->>XKCDCommandHandler: xkcd(ctx, comic_id)
    alt comic_id is not provided
        XKCDCommandHandler->>commands.Context: send_help("xkcd")
        XKCDCommandHandler-->>User: return
    else comic_id is provided
        XKCDCommandHandler->>XKCDCommandHandler: specific(ctx, comic_id)
    end

Class diagram for Sentry initialization optimization in cog loader

classDiagram
    class CogLoader {
        +load_times: dict
        +async _load_single_cog(path: Path)
        +async _load_cog_group(cogs: Sequence[Path])
        +async _process_single_file(path: Path)
        +async _process_directory(path: Path)
        +async load_cogs(path: Path)
        +async load_cogs_from_folder(folder_name: str)
        +static async setup(bot: commands.Bot)
    }
    class sentry_sdk {
        +is_initialized()
        +get_current_span()
    }
    CogLoader ..> sentry_sdk : uses
    class SENTRY_INITIALIZED {
        +bool
    }
    CogLoader ..> SENTRY_INITIALIZED : uses

Class diagram for refactored avatar command handler

classDiagram
    class AvatarCommandHandler {
        +async send_avatar(source, member)
        +async create_avatar_file(avatar)
    }
    class discord.Interaction {
        +response.send_message()
    }
    class commands.Context {
        +author
        +reply()
    }
    AvatarCommandHandler ..> discord.Interaction : interacts
    AvatarCommandHandler ..> commands.Context : interacts

Class diagram for refactored xkcd command handler

classDiagram
    class XKCDCommandHandler {
        +async xkcd(ctx, comic_id)
        +async specific(ctx, comic_id)
    }
    class commands.Context {
        +send_help()
    }
    XKCDCommandHandler ..> commands.Context : interacts

File-Level Changes

Change Details Files
Cache Sentry initialization state and replace repeated checks
  • Introduce module-level SENTRY_INITIALIZED constant
  • Swap all sentry_sdk.is_initialized() calls to use SENTRY_INITIALIZED
  • Move bot.id span tagging under the new constant guard
tux/cog_loader.py
Simplify avatar command control flow
  • Reorder check to handle Interaction sources first
  • Unify message sending to always use source.response.send_message
  • Remove redundant branches for reply vs. response
tux/cogs/info/avatar.py
Refactor xkcd command for early return and clearer branching
  • Invert comic_id guard to return early when missing
  • Shift specific comic call below the guard
  • Eliminate the nested else block for clarity
tux/cogs/fun/xkcd.py

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an issue from a review comment by replying to it. You can also reply to a review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull request title to generate a title at any time. You can also comment @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in the pull request body to generate a PR summary at any time exactly where you want it. You can also comment @sourcery-ai summary on the pull request to (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the pull request to resolve all Sourcery comments. Useful if you've already addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull request to dismiss all existing Sourcery reviews. Especially useful if you want to start fresh with a new review - don't forget to comment @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

  • Contact our support team for questions or feedback.
  • Visit our documentation for detailed guides and information.
  • Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

sourcery-ai[bot] avatar Jul 16 '25 21:07 sourcery-ai[bot]

@sourcery-ai review

amadaluzia avatar Jul 17 '25 21:07 amadaluzia

Codecov Report

:x: Patch coverage is 3.57143% with 27 lines in your changes missing coverage. Please review. :white_check_mark: Project coverage is 9.75%. Comparing base (8cd2837) to head (39ac2e4). :white_check_mark: All tests successful. No failed tests found.

Files with missing lines Patch % Lines
tux/cog_loader.py 4.54% 21 Missing :warning:
tux/cogs/fun/xkcd.py 0.00% 3 Missing :warning:
tux/cogs/info/avatar.py 0.00% 3 Missing :warning:
Additional details and impacted files
@@           Coverage Diff            @@
##            main    #953      +/-   ##
========================================
+ Coverage   9.63%   9.75%   +0.12%     
========================================
  Files        123     123              
  Lines      10414   10412       -2     
  Branches    1279    1277       -2     
========================================
+ Hits        1003    1016      +13     
+ Misses      9307    9280      -27     
- Partials     104     116      +12     
Flag Coverage Δ *Carryforward flag
database 0.31% <ø> (+<0.01%) :arrow_up: Carriedforward from 8cd2837
integration 5.86% <3.57%> (+0.01%) :arrow_up:
unit 6.31% <3.57%> (+0.01%) :arrow_up:

*This pull request uses carry forward flags. Click here to find out more.

Components Coverage Δ
Core Bot Infrastructure 16.52% <4.54%> (+0.08%) :arrow_up:
Database Layer 0.00% <ø> (ø)
Bot Commands & Features 0.00% <0.00%> (ø)
Event & Error Handling ∅ <ø> (∅)
Utilities & Helpers ∅ <ø> (∅)
User Interface Components 0.00% <ø> (ø)
CLI Interface ∅ <ø> (∅)
External Service Wrappers ∅ <ø> (∅)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Jul 17 '25 23:07 codecov[bot]

@sourcery-ai review

amadaluzia avatar Aug 07 '25 17:08 amadaluzia

@sourcery-ai review

amadaluzia avatar Aug 07 '25 18:08 amadaluzia