Fix some issues that ruff found, format with black, use consistent zoom scale#62
Merged
AlanRockefeller merged 6 commits intomainfrom Mar 28, 2026
Merged
Fix some issues that ruff found, format with black, use consistent zoom scale#62AlanRockefeller merged 6 commits intomainfrom
AlanRockefeller merged 6 commits intomainfrom
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughWide-scope import/formatting cleanup across tests and modules, a focused rewrite of highlight-recovery math, a small QML zoom-percent calculation change, tooling config added to pyproject, and refactoring of a test import-check script into functions with a CLI entrypoint. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
faststack/tests/test_highlights_v2.py (1)
45-45: Consider removing the unused variable entirely.The
_srgbvariable is created but never used in the test. While prefixing with_silences the linter warning, consider removing it entirely unless it's genuinely needed for future test expansion.🧹 Proposed removal
- _srgb = np.ones((100, 100, 3), dtype=np.uint8) * 255 -🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@faststack/tests/test_highlights_v2.py` at line 45, Remove the unused test variable _srgb from the test (it's created as "_srgb = np.ones((100, 100, 3), dtype=np.uint8) * 255" in test_highlights_v2.py) — either delete that assignment entirely or replace it with a meaningful use in the test; preferred fix: remove the _srgb line to avoid dead code and silence the linter.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@faststack/tests/test_highlights_v2.py`:
- Line 45: Remove the unused test variable _srgb from the test (it's created as
"_srgb = np.ones((100, 100, 3), dtype=np.uint8) * 255" in test_highlights_v2.py)
— either delete that assignment entirely or replace it with a meaningful use in
the test; preferred fix: remove the _srgb line to avoid dead code and silence
the linter.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 78bd716a-9c6e-45c6-8a24-5c7f5c56d900
📒 Files selected for processing (37)
faststack/app.pyfaststack/imaging/math_utils.pyfaststack/imaging/prefetch.pyfaststack/io/sidecar.pyfaststack/qml/Main.qmlfaststack/tests/debug_app_init.pyfaststack/tests/debug_editor_error.pyfaststack/tests/repro_futures_cleanup.pyfaststack/tests/test_config_setters.pyfaststack/tests/test_delete_worker_edge_cases.pyfaststack/tests/test_deletion_perf_structure.pyfaststack/tests/test_editor_lifecycle_and_safety.pyfaststack/tests/test_editor_no_copy.pyfaststack/tests/test_executor_shutdown.pyfaststack/tests/test_exif_compat.pyfaststack/tests/test_exif_orientation.pyfaststack/tests/test_generation_aware_preview.pyfaststack/tests/test_handle_failures_isolated.pyfaststack/tests/test_highlights_v2.pyfaststack/tests/test_reactive_delete.pyfaststack/tests/test_refresh_crash.pyfaststack/tests/test_refresh_optimization.pyfaststack/tests/test_sidecar.pyfaststack/tests/test_startup_opt.pyfaststack/tests/test_startup_optimization.pyfaststack/tests/test_thumbnail_ready_emits_datachanged.pyfaststack/tests/test_turbo.pyfaststack/tests/test_ui_prefetch_safety.pyfaststack/tests/test_ui_state_recycle.pyfaststack/tests/test_variants.pyfaststack/tests/test_variants_logic.pyfaststack/tests/thumbnail_view/test_model.pyfaststack/tests/thumbnail_view/test_prefetcher_priority.pyfaststack/tests/thumbnail_view/test_reason_tracking.pyfaststack/tests/thumbnail_view/test_thumbnail_ready.pyfaststack/thumbnail_view/model.pyfaststack/thumbnail_view/prefetcher.py
💤 Files with no reviewable changes (13)
- faststack/tests/test_refresh_optimization.py
- faststack/tests/test_startup_opt.py
- faststack/tests/test_config_setters.py
- faststack/tests/test_refresh_crash.py
- faststack/tests/thumbnail_view/test_thumbnail_ready.py
- faststack/tests/thumbnail_view/test_reason_tracking.py
- faststack/tests/test_handle_failures_isolated.py
- faststack/tests/debug_editor_error.py
- faststack/tests/test_executor_shutdown.py
- faststack/tests/test_variants.py
- faststack/tests/test_editor_no_copy.py
- faststack/imaging/math_utils.py
- faststack/tests/test_ui_state_recycle.py
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
faststack/tests/check_imports.py (1)
22-24: Path setup is invocation-directory dependent.Using
os.getcwd()on Line 23 can point to the wrong directory when run outside repo root. Prefer deriving project root from__file__for deterministic behavior.Proposed fix
+from pathlib import Path import os import sys @@ - # Add current directory to path - sys.path.append(os.getcwd()) + # Add repository root to path (stable regardless of invocation cwd) + repo_root = Path(__file__).resolve().parents[2] + if str(repo_root) not in sys.path: + sys.path.insert(0, str(repo_root))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@faststack/tests/check_imports.py` around lines 22 - 24, Replace the invocation-dependent sys.path.append(os.getcwd()) with a deterministic project-root calculation based on this script's location: compute project_root = os.path.abspath(os.path.join(os.path.dirname(__file__), "..")) (or adjust the parent chain as needed) and append that path with sys.path.insert(0, project_root) (or sys.path.append) instead of using os.getcwd(); update the symbol sys.path.append(os.getcwd()) in tests/check_imports.py to use this __file__-derived project_root so imports are correct regardless of invocation directory.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@faststack/tests/check_imports.py`:
- Around line 13-18: The exception handlers inside the import loop (the except
ImportError/except Exception blocks handling imports of module_name) only print
errors so main() can still exit zero; change this to record failures and return
a non-zero exit: add a failures counter or list in the import loop,
increment/append inside both except blocks (or re-raise a SystemExit), and after
the loop in main() call sys.exit(1) (or raise SystemExit) if any failures were
recorded; also print a concise failure summary before exiting so CI fails when
imports break.
---
Nitpick comments:
In `@faststack/tests/check_imports.py`:
- Around line 22-24: Replace the invocation-dependent
sys.path.append(os.getcwd()) with a deterministic project-root calculation based
on this script's location: compute project_root =
os.path.abspath(os.path.join(os.path.dirname(__file__), "..")) (or adjust the
parent chain as needed) and append that path with sys.path.insert(0,
project_root) (or sys.path.append) instead of using os.getcwd(); update the
symbol sys.path.append(os.getcwd()) in tests/check_imports.py to use this
__file__-derived project_root so imports are correct regardless of invocation
directory.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 62a346c8-a3a3-45d2-9b40-d42e9cf1a49d
📒 Files selected for processing (3)
faststack/tests/check_imports.pyfaststack/tests/test_cache_invalidation.pypyproject.toml
✅ Files skipped from review due to trivial changes (1)
- pyproject.toml
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Bug Fixes
Refactor
Chores