Skip to content

Conversation

@yuval-qf
Copy link
Collaborator

@yuval-qf yuval-qf commented Jan 8, 2026

Description

Motivation and Context

Type of Change

  • 🐛 Bug fix (non-breaking change which fixes an issue)
  • ✨ New feature (non-breaking change which adds functionality)
  • 💥 Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • 📝 Documentation update
  • 🎨 Code style/refactoring (no functional changes)
  • 🧪 Test updates
  • 🔧 Configuration/build changes

Changes Made

Screenshots/Examples (if applicable)

Checklist

  • I have read the CONTRIBUTING.md guide
  • My code follows the code style of this project (PEP 8, type hints, docstrings)
  • I have run uv run black . to format my code
  • I have run uv run flake8 . and fixed all issues
  • I have run uv run mypy --config-file .mypy.ini . and addressed type checking issues
  • I have run uv run bandit -c .bandit.yaml -r . for security checks
  • I have added tests that prove my fix is effective or that my feature works
  • I have run uv run pytest and all tests pass
  • I have manually tested my changes
  • I have updated the documentation accordingly
  • I have added/updated type hints for new/modified functions
  • My changes generate no new warnings
  • I have checked my code for security issues
  • Any dependent changes have been merged and published

Testing

Test Configuration:

  • Python version:
  • OS:
  • Other relevant details:

Test Steps:
1.
2.
3.

Additional Notes

Related Issues/PRs

  • Fixes #
  • Related to #
  • Depends on #

@yuval-qf yuval-qf requested a review from drorIvry as a code owner January 8, 2026 10:07
@yuval-qf yuval-qf self-assigned this Jan 8, 2026
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 8, 2026

Summary by CodeRabbit

Release Notes v0.3.3

  • Chores

    • Version bumped to 0.3.3
  • Bug Fixes

    • Fixed stdout logging level handling to respect debug mode settings when file logging is enabled
  • Improvements

    • Enhanced update check logging for better visibility into version checking process

✏️ Tip: You can customize this high-level summary in your review settings.

Walkthrough

A minor version bump to 0.3.3 with enhancements to logging visibility across the update checker, TUI installer, and logging configuration. The GitHub API endpoint for fetching releases was refactored to use conditional paths for latest vs. tagged versions. Logging statements were added throughout to improve runtime observability without altering core logic.

Changes

Cohort / File(s) Summary
Version Management
VERSION
Version string bumped from 0.3.2 to 0.3.3
Logging Configuration
rogue/common/logging/config.py
Stdout logging level now conditional on debug flag when file_path is provided: remains unchanged if debug is True, otherwise set to ERROR
TUI Installer
rogue/common/tui_installer.py
GitHub API endpoint refactored to use /releases with conditional branching (latest vs. /tags/{version}); added debug log statement in version check
Update Checker
rogue/common/update_checker.py
Added four logging statements for update check flow visibility (skip notice, version comparison, missing version, new version detection)

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • PR #131: Modifies VERSION file with version string bump (same file impacted)
  • PR #143: Modifies tui_installer logic with GitHub release endpoint and version check enhancements

Suggested reviewers

  • drorIvry

Poem

🐰 A version hops from point-two to point-three,
With logging so bright the update flows free,
GitHub endpoints now split their path,
Latest or tagged—each takes its own path,
Debug whispers softly through the TUI's release!

🚥 Pre-merge checks | ✅ 1 | ❌ 2
❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description is entirely unfilled—only the template structure remains with no actual content, motivation, type of change selection, or test information provided. Complete the description by filling in all sections: description of changes, motivation/context, type of change selection, specific changes made, and testing information.
Docstring Coverage ⚠️ Warning Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed Title accurately describes the main changes: a TUI download fix and additional debug logging across multiple files.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

📜 Recent review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 48c29f5 and f0ed1b9.

📒 Files selected for processing (4)
  • VERSION
  • rogue/common/logging/config.py
  • rogue/common/tui_installer.py
  • rogue/common/update_checker.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

**/*.py: Use black for code formatting
Use type hints for all function signatures
Follow PEP 8 naming conventions (snake_case for variables and functions, PascalCase for classes)
Use try/except blocks for code that may raise exceptions
Use isort conventions for import organization

Files:

  • rogue/common/update_checker.py
  • rogue/common/logging/config.py
  • rogue/common/tui_installer.py
🔇 Additional comments (7)
VERSION (1)

1-1: LGTM! Version bump for hotfix release.

The version increment to 0.3.3 is consistent with the hotfix nature of this PR.

rogue/common/logging/config.py (1)

87-88: LGTM! Improved stdout logging behavior in debug mode.

The conditional logic correctly preserves the debug level for stdout when debug=True, rather than always forcing it to ERROR. This makes debug mode more useful when logging to a file, as debug messages will now appear on stdout when explicitly requested.

The behavior is now:

  • debug=True, file_path set: stdout and file both get DEBUG level
  • debug=False, file_path set: stdout gets ERROR, file gets INFO
rogue/common/update_checker.py (3)

38-38: LGTM! Helpful debug logging for update check flow.

The debug log provides visibility into when update checks are skipped due to recent cache hits.


43-46: LGTM! Version comparison logging adds useful observability.

Logging both current and latest versions (and the absence of a latest version) helps diagnose update check behavior in production.


54-56: LGTM! Clear logging when updates are available.

The debug message clearly indicates when a newer version is detected, which is helpful for troubleshooting.

rogue/common/tui_installer.py (2)

213-213: LGTM! Useful debug logging for version comparison.

The debug log provides visibility into the installed TUI version before the comparison logic, which helps troubleshoot reinstallation decisions.


56-61: GitHub API endpoints are correct, but the error handling description is inaccurate.

The URL construction properly aligns with GitHub's REST API documentation:

  • GET /repos/{owner}/{repo}/releases/latest for latest releases
  • GET /repos/{owner}/{repo}/releases/tags/{version} for tagged versions

However, lines 75-77 do not implement fallback logic as described. The except block only catches exceptions, logs them, and returns None. The actual fallback to "latest" version is handled at a higher level in the calling code (_download_rogue_tui_to_temp method), which wraps this method in its own try/except and retries with latest_version_override=True. Lines 75-77 are simply the lowest-level error suppression, not the fallback mechanism.

Likely an incorrect or invalid review comment.


Comment @coderabbitai help to get the list of available commands and usage tips.

@yuval-qf yuval-qf enabled auto-merge (squash) January 8, 2026 10:09
@yuval-qf yuval-qf disabled auto-merge January 8, 2026 10:11
@yuval-qf yuval-qf enabled auto-merge (squash) January 8, 2026 10:17
@yuval-qf yuval-qf merged commit 53e4434 into main Jan 8, 2026
9 checks passed
@yuval-qf yuval-qf deleted the feature/fire-992-debug-logs-and-bugfix branch January 8, 2026 10:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants