Release v0.7 — more improvements#12
Conversation
WalkthroughVersion 0.7 release including JPEG decoding flag removal (TJFLAG_FASTDCT), significant Prefetcher rework with QImage/Pillow fallback and generation-based caching, new UI features (drag-outside-window, Photoshop integration, clipboard support), enhanced error logging, Qt6 color space support, and removal of developer documentation (WARP.md). Changes
Sequence Diagram(s)sequenceDiagram
participant UI as UI/Main
participant Prefetch as Prefetcher
participant Cache as Cache
participant QImage as QImage
participant Pillow as Pillow
participant Storage as Storage
UI->>Prefetch: update_prefetch(current_index)
activate Prefetch
Prefetch->>Prefetch: increment generation
Prefetch->>Prefetch: compute prefetch window
Prefetch->>Prefetch: cancel stale futures
Prefetch->>Prefetch: submit_task(index)
Prefetch->>Prefetch: _decode_and_cache(image_file, index, generation)
activate Prefetch
Note over Prefetch: Check generation snapshot
Prefetch->>QImage: load image file
alt QImage Success
Prefetch->>QImage: optional resize to display dims
QImage-->>Prefetch: RGB888 buffer
else QImage Fails
Prefetch->>Pillow: fallback JPEG decode
alt Pillow Success
Pillow-->>Prefetch: RGB buffer
Prefetch->>Prefetch: optional downscale
else Pillow Fails
Prefetch-->>Prefetch: log error, skip cache
end
end
Prefetch->>Prefetch: re-check generation
alt Generation Still Valid
Prefetch->>Storage: create NumPy buffer
Prefetch->>Cache: cache_put(DecodedImage)
Cache-->>Storage: store in cache
else Generation Stale
Prefetch-->>Prefetch: skip caching
end
deactivate Prefetch
deactivate Prefetch
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Areas requiring extra attention:
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 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.
Actionable comments posted: 2
🧹 Nitpick comments (5)
.gitignore (1)
25-26: Verify necessity of WARP.md in .gitignore.Per the PR objectives, WARP.md is being permanently removed from the repository. If the file is not being regenerated at runtime, adding it to
.gitignoreis unnecessary. Consider removing this entry if the file is not expected to be recreated.faststack/faststack/io/helicon.py (1)
69-69: Remove redundant exception object from log message.When using
log.exception(), the exception details are automatically captured in the stack trace, so including{e}in the message is redundant.Apply this diff:
- log.exception(f"Invalid helicon args format: {e}") + log.exception("Invalid helicon args format")Based on static analysis hints.
faststack/faststack/tests/test_executable_validator.py (1)
65-68: Test correctly validates the allow_custom_paths behavior.The test properly verifies that when
_is_subpathreturns False (executable not in safe directory), validation passes with a warning whenallow_custom_paths=True(the default).However, the unpacked
errorvariable is unused. Apply this diff to follow convention:- is_valid, error = validate_executable_path(suspicious_path) + is_valid, _ = validate_executable_path(suspicious_path)Based on static analysis hints.
faststack/faststack/imaging/prefetch.py.bak (1)
1-127: Remove backup file from version control.This
.bakfile appears to be a backup of the previous implementation. Backup files should typically be excluded from version control (e.g., via.gitignore) to keep the repository clean. Consider removing this file from the commit.faststack/faststack/qml/Components.qml (1)
90-136: Outside-drag logic looks solid; consider future configurabilityThe thresholded outside-drag detection and the guard with
isDraggingOutsidepreserve normal panning and cleanly hand off tocontroller.start_drag_current_image(). This looks correct and side-effect free. If you ever want to tune UX per-platform, you might consider makingdragThresholda configurable constant or value exposed from Python/QML state, but it’s fine as-is.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
.gitignore(1 hunks)WARP.md(0 hunks)faststack/ChangeLog.md(0 hunks)faststack/README.md(1 hunks)faststack/faststack.egg-info/PKG-INFO(4 hunks)faststack/faststack/imaging/jpeg.py(4 hunks)faststack/faststack/imaging/prefetch.py(3 hunks)faststack/faststack/imaging/prefetch.py.bak(1 hunks)faststack/faststack/io/executable_validator.py(2 hunks)faststack/faststack/io/helicon.py(1 hunks)faststack/faststack/qml/Components.qml(1 hunks)faststack/faststack/qml/Main.qml(2 hunks)faststack/faststack/tests/test_executable_validator.py(1 hunks)faststack/faststack/ui/provider.py(2 hunks)
💤 Files with no reviewable changes (2)
- faststack/ChangeLog.md
- WARP.md
🧰 Additional context used
🧬 Code graph analysis (2)
faststack/faststack/imaging/prefetch.py (2)
faststack/faststack/models.py (2)
ImageFile(8-12)DecodedImage(33-42)faststack/faststack/app.py (1)
get_display_info(160-163)
faststack/faststack/tests/test_executable_validator.py (1)
faststack/faststack/io/executable_validator.py (1)
validate_executable_path(23-95)
🪛 LanguageTool
faststack/README.md
[style] ~3-~3: Some style guides suggest that commas should set off the year in a month-day-year date.
Context: ...FastStack # Version 0.7 - November 20, 2025 # By Alan Rockefeller Ultra-fast, cach...
(MISSING_COMMA_AFTER_YEAR)
🪛 Ruff (0.14.5)
faststack/faststack/io/helicon.py
69-69: Redundant exception object included in logging.exception call
(TRY401)
faststack/faststack/ui/provider.py
54-54: Do not catch blind exception: Exception
(BLE001)
faststack/faststack/imaging/prefetch.py
180-180: Consider moving this statement to an else block
(TRY300)
faststack/faststack/tests/test_executable_validator.py
66-66: Unpacked variable error is never used
Prefix it with an underscore or any other dummy variable pattern
(RUF059)
🔇 Additional comments (22)
faststack/README.md (1)
3-3: LGTM! Version bump looks good.The version update from 0.4 to 0.7 and date change are consistent with the release PR objectives.
faststack/faststack/io/executable_validator.py (2)
47-47: Good improvement to error logging.Switching to
log.exception()properly captures the stack trace for path validation errors, making debugging easier.
91-92: Excellent refinement of exception handling.Narrowing the exception types to
(ValueError, OSError)and usinglog.exception()improves both specificity and diagnostics.faststack/faststack.egg-info/PKG-INFO (3)
24-24: Version header is consistent.The version header correctly reflects version 0.7 and the release date. This content should be maintained in your README.md file, from which PKG-INFO will be automatically generated during the build process.
41-45: New features are well-documented.The new features are clearly described and consistent with the keyboard shortcuts section below. The documentation provides good visibility into the version 0.7 enhancements.
70-72: Keyboard shortcuts are clearly documented.The new keyboard shortcuts are well-documented and consistent with the features described earlier in the file.
faststack/faststack/imaging/prefetch.py (10)
1-6: LGTM!The updated docstring clearly documents the architectural shift to QImage-based decoding with Pillow fallback.
13-15: LGTM!The imports correctly support the new QImage-based decoding approach with NumPy buffer handling.
23-41: LGTM!The thread pool sizing (2x CPU cores, capped at 8) is appropriate for I/O-bound image decoding operations.
45-49: LGTM!The method correctly cancels outstanding work when the image list changes, preventing stale prefetch tasks.
78-79: LGTM!Good defensive check to prevent IndexError when accessing
self.image_files[index]on Line 81.
97-115: LGTM!The generation-based staleness check prevents wasted work on outdated prefetch tasks.
118-157: LGTM!The dual-path loading strategy is well-implemented:
- Pillow fallback (lines 121-135): Correctly handles QImage failures, with optional thumbnail downscaling and proper RGB conversion
- QImage path (lines 137-157): Properly handles bytes-per-line padding (line 152) and copies exactly
h * bytes_per_linebytes (line 157)- Memory safety: Both paths use
.copy()(lines 135, 157) to ensure NumPy owns the memory after QImage goes out of scope
159-165: LGTM!The generation re-check before caching prevents race conditions where the display context changes while a decode is in-flight.
167-180: LGTM!The
DecodedImageis correctly populated with the NumPy array buffer (which supports the buffer protocol), dimensions, and bytes-per-line. The cache key includesdisplay_generationto ensure proper cache invalidation.
182-185: LGTM!The addition of
exc_info=Trueon Line 183 is an excellent improvement for debugging - it will capture full stack traces when image decoding fails.Note: The Ruff TRY300 hint suggesting an
elseblock is a false positive. The current structure is correct: Line 180 returns early on success (inside the try block), so the finalreturn Noneis only reached when an exception occurs.faststack/faststack/ui/provider.py (1)
80-83: UIState signal/property additions are consistent and QML-friendlyThe new signals (
statusMessageChanged,resetZoomPanRequested,stackSummaryChanged,filterStringChanged) and thestatusMessage,filterString, andcurrentDirectoryproperties, plusresetZoomPan()emittingresetZoomPanRequested, are all wired consistently with the existing pattern (private backing field where needed, change-only emits, controller delegation elsewhere). This should integrate cleanly with the QML footer and zoom-reset logic.Also applies to: 92-92, 174-203, 291-298
faststack/faststack/qml/Main.qml (1)
133-145: Window-drag fix and label layout tweaks look correctRefreshing
lastMousePoson every move fixes the drifting/accelerating drag behavior for the frameless window, and the directory label’s top alignment + margin is a straightforward visual improvement. No issues from a logic/behavior standpoint.Also applies to: 206-215
faststack/faststack/imaging/jpeg.py (4)
13-13: LGTM: Clean removal of unused import.The removal of
TJFLAG_FASTDCTfrom the import statement is correct since it's no longer used in the codebase.
55-60: LGTM: Consistent color space handling in thumbnail decoding.The
flags=0change is applied consistently with the same quality-over-speed rationale. Thumbnail decoding will now use accurate DCT, ensuring color fidelity even for scaled-down images.
115-120: LGTM: Consistent color space handling in resized decoding.The
flags=0change maintains consistency across all decoding paths. Combined with the explicitpixel_format=TJPF_RGBand scaling factor logic, this ensures high-quality color-accurate output for resized images.
26-28: Verified: PyTurboJPEG decode API supports the usage pattern correctly.The
flags=0withpixel_format=TJPF_RGBapproach provides proper YCbCr→RGB conversion using the standard (more accurate) algorithm. PyTurboJPEG is confirmed as a project dependency infaststack/pyproject.tomlandfaststack/requirements.txt, and the code correctly importsTJPF_RGBfrom the turbojpeg module. The removal ofTJFLAG_FASTDCTin favor offlags=0appropriately prioritizes color accuracy over decoding speed, with proper Pillow fallback error handling in place. The implementation is correct and ready.
| Metadata-Version: 2.4 | ||
| Name: faststack | ||
| Version: 0.6 | ||
| Version: 0.7 |
There was a problem hiding this comment.
Version bump is correct, but PKG-INFO should not be manually edited.
The version bump to 0.7 is consistent with the release. However, PKG-INFO is auto-generated during package builds and this file should not be manually edited or committed to version control.
The faststack.egg-info/ directory is a build artifact that should be excluded from version control. Instead, update the version in your source files (e.g., setup.py, pyproject.toml, or __init__.py) and let the build process regenerate PKG-INFO.
Add to .gitignore:
*.egg-info/
Then remove the directory from the repository:
git rm -r faststack/faststack.egg-info/| # Try to import QColorSpace if available (Qt 6+) | ||
| try: | ||
| from PySide6.QtGui import QColorSpace | ||
| HAS_COLOR_SPACE = True | ||
| except ImportError: | ||
| HAS_COLOR_SPACE = False | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
Defensive color-space handling is good; consider narrowing the broad exception
The optional QColorSpace import + HAS_COLOR_SPACE gate is a nice, low-risk way to add sRGB tagging. The try/except around fromNamedColorSpace / setColorSpace prevents crashes from Qt internals, but the blanket except Exception is what Ruff flags (BLE001). If you want to quiet that without losing safety, you could either:
- Catch a narrower set of expected runtime errors (e.g.,
RuntimeError,ValueError) or - Keep
except Exceptionbut log withexc_info=Trueand explicitly annotate the rationale in a comment for lint suppression.
Functionally this is fine; this is just about lint/style and debuggability.
Also applies to: 45-57
---
🏁 Script executed:
```shell
#!/bin/bash
# Locate the provider.py file and examine the specific lines
fd -t f "provider\.py" -x cat {}
Length of output: 17622
🏁 Script executed:
#!/bin/bash
# Show lines 10-16 and 45-57 with context
fd -t f "provider\.py" -x sed -n '5,20p;40,65p' {}Length of output: 2975
🏁 Script executed:
#!/bin/bash
# Check if the file exists at the expected path
test -f faststack/faststack/ui/provider.py && echo "File found" || echo "File not found"
cat faststack/faststack/ui/provider.py | head -70Length of output: 2523
Now I need to verify what exceptions could be raised by the PySide6 QColorSpace methods to inform whether narrowing the exception is feasible and what specific types to catch.
---
🌐 Web query:
```web
PySide6 QColorSpace.fromNamedColorSpace exceptions raised
💡 Result:
There is no documented static method named fromNamedColorSpace in PySide6’s QColorSpace API — create a named colorspace with the constructor instead (QColorSpace(QColorSpace.NamedColorSpace.)). The official PySide6/Qt docs show the constructors and the NamedColorSpace enum but do not list fromNamedColorSpace. If you call a missing/incorrect API you’ll get a typical Python error (e.g. AttributeError if you call QColorSpace.fromNamedColorSpace, or TypeError if you pass a wrong type to the constructor). See the QColorSpace docs and NamedColorSpace enum. [1][2]
Sources:
[1] PySide6.QtGui.QColorSpace documentation. [2] Qt QColorSpace class / NamedColorSpace enum. (doc.qt.io)
Citations:
The fromNamedColorSpace method does not exist—use the constructor instead
The official PySide6 QColorSpace API does not have a fromNamedColorSpace static method. The current code at lines 47–50 will raise AttributeError at runtime on every image load, silently caught and logged as a warning. The sRGB color space will never actually be applied.
Fix: Use the constructor instead:
cs = QColorSpace(QColorSpace.NamedColorSpace.SRgb)After this fix, you can narrow the exception to catch only expected errors (e.g., RuntimeError, ValueError) or remove the try/except entirely if the constructor is reliable.
🤖 Prompt for AI Agents
In faststack/faststack/ui/provider.py around lines 10 to 16, the code tries to
call QColorSpace.fromNamedColorSpace which doesn't exist and will raise
AttributeError; replace that call with constructing the sRGB space via the
QColorSpace constructor using the NamedColorSpace enum (i.e. create a
QColorSpace instance for SRgb), and then narrow or remove the surrounding
try/except so you only catch expected exceptions (RuntimeError/ValueError)
rather than masking AttributeError.
1 participant
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.