Test#18
Conversation
WalkthroughThis PR introduces FastStack, a comprehensive desktop image viewer and editor application with performance-optimized JPEG decoding, intelligent prefetching, advanced image organization (stacks/batches), external software integrations (Helicon Focus, Photoshop), built-in image editing with ICC color management, and a full Qt/QML-based graphical interface. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant QML as QML UI
participant AppCtrl as AppController
participant Cache as ByteLRUCache
participant Prefetch as Prefetcher
participant Decode as JPEG Decoder
participant Provider as ImageProvider
User->>QML: Click next image
QML->>AppCtrl: next_image()
AppCtrl->>Cache: Check cache for decoded[next_idx]
Cache-->>AppCtrl: Cache hit or miss
alt Cache hit
AppCtrl->>Provider: Get from cache
else Cache miss
AppCtrl->>Decode: decode_jpeg_resized(bytes, W, H)
Decode-->>AppCtrl: RGB numpy array
AppCtrl->>Cache: Store in ByteLRUCache
end
AppCtrl->>Prefetch: update_prefetch(next_idx, direction=+1)
Prefetch->>Decode: Submit background decode tasks<br/>for next_idx±radius
AppCtrl->>QML: Emit currentImageSourceChanged
QML->>Provider: requestImage(next_idx)
Provider-->>QML: QImage (from cache)
QML-->>User: Display image
sequenceDiagram
participant User
participant EditorUI as Image Editor Dialog
participant AppCtrl as AppController
participant ImgEditor as ImageEditor
participant Decode as JPEG/Pillow
User->>EditorUI: Open image editor
EditorUI->>AppCtrl: dialog_opened()
AppCtrl->>ImgEditor: load_image(path)
ImgEditor->>Decode: Load and cache preview
ImgEditor-->>AppCtrl: Success
User->>EditorUI: Move brightness slider
EditorUI->>AppCtrl: set_edit_parameter("brightness", value)
AppCtrl->>ImgEditor: set_edit_param("brightness", value)
ImgEditor->>ImgEditor: _apply_edits() pipeline
ImgEditor-->>AppCtrl: Updated preview data
AppCtrl->>EditorUI: Emit brightnessChanged signal
EditorUI-->>User: Live preview update
User->>EditorUI: Save changes
EditorUI->>AppCtrl: save_edited_image()
AppCtrl->>ImgEditor: save_image()
ImgEditor->>ImgEditor: Create backup (-backup, -backup2, ...)
ImgEditor->>Decode: Save edited as PNG/JPEG
ImgEditor-->>AppCtrl: Success
AppCtrl->>AppCtrl: Reload current image
AppCtrl-->>EditorUI: Notify UI
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Areas requiring extra attention:
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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: 26
♻️ Duplicate comments (1)
faststack.working-menus/faststack/ui/provider.py (1)
28-36: SamerequestImageunused-argument concern as in main provider.
sizeandrequestedSizeare required by the QQuickImageProvider API but not used. As in the main provider module, consider renaming them to_size/_requestedSize(or adding a short comment) to document intentional non-use and satisfy linters.
🧹 Nitpick comments (43)
faststack.working-menus/faststack.egg-info/dependency_links.txt (1)
1-1: Consider excluding generated egg-info artifacts from version control.This file is part of the auto-generated
egg-infopackaging metadata. While the empty content is correct and expected, consider whether these generated artifacts should be committed to the repository or added to.gitignoreinstead. Most projects exclude build artifacts and packaging metadata to keep the repository clean and focused on source code.If you'd like, I can help identify all generated artifacts and suggest a
.gitignoreconfiguration to exclude them from version control.faststack/pyproject.toml (1)
20-28: Consider moving pytest to optional dependencies.pytest is listed as a runtime dependency but is typically only needed for development and testing. Consider moving it to an optional dependency group.
Apply this pattern:
dependencies = [ "PySide6>=6.0,<7.0", "PyTurboJPEG>=1.8,<2.0", "numpy>=2.0,<3.0", "cachetools>=5.0,<6.0", "watchdog>=4.0,<5.0", "Pillow>=10.0,<11.0", - "pytest>=8.0,<9.0", ] + +[project.optional-dependencies] +dev = [ + "pytest>=8.0,<9.0", +]faststack/ChangeLog.md (1)
5-9: Two version releases on the same date.Versions 1.2.0 and 1.1.0 are both dated 2025-11-22. While this can happen with rapid iteration, ensure the version numbers accurately reflect the release sequence and consider whether these should be separate releases or consolidated.
faststack.working-menus/faststack.egg-info/PKG-INFO (1)
1-72: Consider excluding auto-generated packaging metadata from version control.The
egg-infodirectory and its contents are auto-generated during package installation and builds. These files should typically be excluded from version control by adding*.egg-info/to.gitignoreto avoid merge conflicts and ensure metadata stays in sync with the build process.faststack.working-menus/faststack/io/indexer.py (2)
39-40: Remove unused exception variable.The exception variable
eis captured but never used. Sincelog.exception()automatically includes exception details, the variable binding is redundant.Apply this diff:
- except OSError as e: + except OSError: log.exception("Error scanning directory %s", directory)
64-68: Consider removing unused parameter.The
jpg_pathparameter is not used in the function body. If it's not needed for future extensibility, consider removing it to keep the signature clean.Apply this diff:
def _find_raw_pair( - jpg_path: Path, jpg_stat: os.stat_result, potential_raws: List[Tuple[Path, os.stat_result]] ) -> Path | None:And update the call site at line 48:
- raw_pair = _find_raw_pair(jpg_path, jpg_stat, raws.get(jpg_path.stem, [])) + raw_pair = _find_raw_pair(jpg_stat, raws.get(jpg_path.stem, []))faststack.working-menus/faststack/imaging/jpeg.py (2)
29-30: Remove redundant exception details from logging.exception calls.The
logging.exception()method automatically includes exception details and traceback. Including the exception in the message string (e.g.,f"... {e}") is redundant.Apply these diffs:
- log.exception(f"PyTurboJPEG failed to decode image: {e}. Trying Pillow.") + log.exception("PyTurboJPEG failed to decode image. Trying Pillow.")- log.exception(f"Pillow also failed to decode image: {e}") + log.exception("Pillow also failed to decode image")- log.exception(f"PyTurboJPEG failed to decode thumbnail: {e}. Trying Pillow.") + log.exception("PyTurboJPEG failed to decode thumbnail. Trying Pillow.")- log.exception(f"Pillow also failed to decode thumbnail: {e}") + log.exception("Pillow also failed to decode thumbnail")- log.exception(f"PyTurboJPEG failed: {e}") + log.exception("PyTurboJPEG failed")- log.exception(f"Pillow failed to decode and resize image: {e}") + log.exception("Pillow failed to decode and resize image")Also applies to: 38-39, 66-67, 75-76, 129-130, 147-148
61-65: Consider using else block for clearer control flow.The successful return at line 65 could be placed in an
elseblock for better structure, making it clear that it only executes when no exception occurs.Apply this diff:
decoded = jpeg_decoder.decode( jpeg_bytes, scaling_factor=scaling_factor, pixel_format=TJPF_RGB, flags=0, # Proper color space handling ) if decoded.shape[0] > max_dim or decoded.shape[1] > max_dim: img = Image.fromarray(decoded) img.thumbnail((max_dim, max_dim), Image.Resampling.LANCZOS) return np.array(img) - return decoded + else: + return decoded except Exception as e: log.exception(f"PyTurboJPEG failed to decode thumbnail: {e}. Trying Pillow.")faststack.working-menus/faststack/tests/test_pairing.py (4)
55-55: Split multiple statements onto separate lines.Multiple statements on one line reduce readability and violate PEP 8 conventions.
Apply this diff:
- jpg_stat = MagicMock(); jpg_stat.st_mtime = 1000.0 + jpg_stat = MagicMock() + jpg_stat.st_mtime = 1000.0
58-58: Split multiple statements onto separate lines.Apply this diff:
- raw1_path = Path("IMG_01.CR3"); raw1_stat = MagicMock(); raw1_stat.st_mtime = 1000.1 + raw1_path = Path("IMG_01.CR3") + raw1_stat = MagicMock() + raw1_stat.st_mtime = 1000.1
63-63: Split multiple statements onto separate lines.Apply this diff:
- raw2_path = Path("IMG_01.CR3"); raw2_stat = MagicMock(); raw2_stat.st_mtime = 1003.0 + raw2_path = Path("IMG_01.CR3") + raw2_stat = MagicMock() + raw2_stat.st_mtime = 1003.0
68-69: Split multiple statements onto separate lines.Apply this diff:
- raw3_path = Path("IMG_01_A.CR3"); raw3_stat = MagicMock(); raw3_stat.st_mtime = 1000.5 - raw4_path = Path("IMG_01_B.CR3"); raw4_stat = MagicMock(); raw4_stat.st_mtime = 1001.8 + raw3_path = Path("IMG_01_A.CR3") + raw3_stat = MagicMock() + raw3_stat.st_mtime = 1000.5 + raw4_path = Path("IMG_01_B.CR3") + raw4_stat = MagicMock() + raw4_stat.st_mtime = 1001.8faststack.working-menus/faststack/io/sidecar.py (3)
57-60: Uselogging.exceptionfor better diagnostics.When logging from an exception handler,
logging.exceptionautomatically includes the traceback, which aids in debugging.Apply this diff:
- except (json.JSONDecodeError, TypeError) as e: - log.error(f"Failed to load or parse sidecar file {self.path}: {e}") + except (json.JSONDecodeError, TypeError): + log.exception(f"Failed to load or parse sidecar file {self.path}")
87-88: Uselogging.exceptionfor better diagnostics.Apply this diff:
- except (IOError, TypeError) as e: - log.error(f"Failed to save sidecar file {self.path}: {e}") + except (IOError, TypeError): + log.exception(f"Failed to save sidecar file {self.path}")
75-78: Consider usingdataclasses.asdict()for more robust serialization.Using
meta.__dict__directly assumes all fields are in__dict__, which is true for standard dataclasses but could break if slots or other customizations are added later.dataclasses.asdict()provides a more robust approach.Apply this diff to use the dataclasses library:
+import dataclasses + # ... in save() method: "entries": { - stem: meta.__dict__ + stem: dataclasses.asdict(meta) for stem, meta in self.data.entries.items() },faststack.working-menus/faststack/tests/test_sidecar.py (1)
14-14: Use explicitOptionaltype hint.PEP 484 prohibits implicit
Optional. Specify the type explicitly.Apply this diff:
- def _create(content: dict = None): + def _create(content: Optional[dict] = None):faststack/faststack/qml/ImageEditorDialog.qml.new (1)
176-214: Fix inconsistent indentation (tabs vs spaces).The slider
backgroundandhandlesections use tabs for indentation, while the rest of the file uses spaces. Consistent indentation improves readability and avoids potential issues with some editors and linters.Convert tabs to spaces to match the rest of the file:
- background: Rectangle { - implicitHeight: 8 + background: Rectangle { + implicitHeight: 8(Apply similar spacing fixes to all lines 176-214)
faststack/faststack/io/helicon.py (1)
70-72: Uselog.errorinstead oflog.exceptionfor ValueError.
ValueErrorfromshlex.splitis a validation error (invalid argument format), not an unexpected exception that requires a full traceback. Usinglog.erroris more appropriate here.Apply this diff:
except ValueError as e: - log.exception(f"Invalid helicon args format: {e}") + log.error(f"Invalid helicon args format: {e}") return False, Nonefaststack.working-menus/faststack/logging_setup.py (2)
8-13: Consider using a dedicated library for cross-platform data directories.The current implementation only checks
APPDATA(Windows) and falls back to~/.faststack. For better cross-platform support, consider using theplatformdirslibrary which handles Windows, macOS, and Linux conventions properly.Example refactor:
def get_app_data_dir() -> Path: """Returns the application data directory.""" try: from platformdirs import user_data_dir return Path(user_data_dir("faststack", "faststack")) except ImportError: # Fallback to current implementation app_data = os.getenv("APPDATA") if app_data: return Path(app_data) / "faststack" return Path.home() / ".faststack"Add to your dependencies:
# In pyproject.toml or requirements.txt platformdirs>=3.0.0
15-46: Consider using a named logger instead of configuring the root logger.Configuring the root logger (line 33-36) can affect third-party libraries' logging behavior. Consider creating a named logger for the FastStack application to avoid side effects.
Example refactor:
def setup_logging(debug: bool = False): """Sets up logging to a rotating file in the app data directory. Args: debug: If True, sets log level to DEBUG. Otherwise, sets to INFO to reduce noise. """ log_dir = get_app_data_dir() / "logs" log_dir.mkdir(parents=True, exist_ok=True) log_file = log_dir / "app.log" handler = logging.handlers.RotatingFileHandler( log_file, maxBytes=10*1024*1024, backupCount=5 ) formatter = logging.Formatter( "%(asctime)s - %(name)s - %(levelname)s - %(message)s" ) handler.setFormatter(formatter) # Use named logger for faststack instead of root app_logger = logging.getLogger("faststack") app_logger.setLevel(logging.DEBUG if debug else logging.INFO) app_logger.addHandler(handler) # Configure logging for specific modules if debug: logging.getLogger("faststack.imaging.cache").setLevel(logging.DEBUG) logging.getLogger("faststack.imaging.prefetch").setLevel(logging.DEBUG) else: logging.getLogger("faststack.imaging.cache").setLevel(logging.ERROR) logging.getLogger("faststack.imaging.prefetch").setLevel(logging.ERROR) logging.getLogger("PIL").setLevel(logging.INFO)faststack.working-menus/faststack/tests/test_executable_validator.py (5)
40-40: Fix unused lambda argument.The
selfparameter in the lambda is unused. Use_to indicate it's intentionally ignored.Apply this diff:
- mock_path_instance.__str__ = lambda self: photoshop_path + mock_path_instance.__str__ = lambda _: photoshop_path
62-62: Fix unused lambda argument.The
selfparameter in the lambda is unused. Use_to indicate it's intentionally ignored.Apply this diff:
- mock_path_instance.__str__ = lambda self: r"C:\Windows\System32\malware.exe" + mock_path_instance.__str__ = lambda _: r"C:\Windows\System32\malware.exe"
66-66: Prefix unused variable with underscore.The
errorvariable is unpacked but never used. Prefix it with_to indicate it's intentionally unused.Apply this diff:
- is_valid, error = validate_executable_path(suspicious_path) + is_valid, _ = validate_executable_path(suspicious_path)
123-123: Fix unused lambda argument.The
selfparameter in the lambda is unused. Use_to indicate it's intentionally ignored.Apply this diff:
- mock_path_instance.__str__ = lambda self: wrong_exe + mock_path_instance.__str__ = lambda _: wrong_exe
127-131: Prefix unused variable with underscore.The
errorvariable is unpacked but never used. Prefix it with_to indicate it's intentionally unused.Apply this diff:
- is_valid, error = validate_executable_path( + is_valid, _ = validate_executable_path( wrong_exe, app_type="photoshop" )faststack.working-menus/faststack/qml/FilterDialog.qml (1)
63-77: Ensure consistent null-safety checks for controller methods.Line 65 uses optional chaining for
controller.get_filter_string, but lines 71 and 76 callcontroller.dialog_opened()andcontroller.dialog_closed()without checking if they exist. This inconsistency could lead to runtime errors if the controller doesn't implement these methods.Apply consistent error handling:
onOpened: { // Load current filter string from controller var current = controller.get_filter_string ? controller.get_filter_string() : "" filterDialog.filterString = current || "" filterField.text = filterDialog.filterString filterField.forceActiveFocus() filterField.selectAll() // Notify Python that a dialog is open - controller.dialog_opened() + if (controller.dialog_opened) controller.dialog_opened() } onClosed: { // Notify Python that dialog is closed - controller.dialog_closed() + if (controller.dialog_closed) controller.dialog_closed() }faststack/faststack/qml/ImageEditorDialog.qml (1)
156-179: Slider ↔ backend synchronization is correct but could be simplifiedThe slider uses three mechanisms to stay in sync with the backend:
value: backendValue,onBackendValueChanged, and theConnectionshandler onupdatePulseChanged, all of which can assign toslider.value. This works, but it’s harder to reason about which path is responsible for which update, and direct assignments also break the original binding onvalue.If you start seeing subtle sync bugs later, consider simplifying to a single source of truth—for example:
- Drop the initial
value: backendValuebinding and rely solely on the explicit handlers, or- Keep the binding and move the “force refresh even when the numeric value is unchanged” behavior into a dedicated pulse-based binding/helper.
Not urgent, but a potential future cleanup to reduce cognitive load.
Also applies to: 181-189, 201-207
faststack.working-menus/faststack/io/helicon.py (1)
16-24: Docstring return description is out of sync with the actual return typeThe type hint and implementation return
(bool, Optional[Path]), but the docstring only documents a singlebool(“True if the process was launched successfully, False otherwise.”).Recommend updating the docstring to describe the tuple, e.g. that it returns
(success, tmp_list_path)where the second element is the path to the temp file (orNoneon failure).faststack.working-menus/faststack/imaging/cache.py (1)
10-21: Tightenget_decoded_image_sizedocstring and fallback behaviorThe cache wrapper itself looks good and matches how
cachetools.LRUCacheexpectsmaxsize/getsizeofto be used.For
get_decoded_image_size:
- The docstring still refers to “a decoded image tuple (buffer, qimage)”, but the implementation now works with
faststack.models.DecodedImage.- Returning
1for non‑DecodedImageitems silently underestimates their size, which can effectively disable byte‑limit enforcement if this helper is reused elsewhere.Consider updating both the docstring and the fallback behavior. For example:
-def get_decoded_image_size(item) -> int: - """Calculates the size of a decoded image tuple (buffer, qimage).""" +def get_decoded_image_size(item) -> int: + """Calculates the size in bytes of a decoded image (or falls back to `__sizeof__`).""" @@ - if isinstance(item, DecodedImage): + if isinstance(item, DecodedImage): @@ - return item.width * item.height * 3 - return 1 # Should not happen + return item.width * item.height * 3 + + # Fallback: defer to Python's size estimator so we don't silently under-account. + try: + return item.__sizeof__() # type: ignore[no-any-return] + except AttributeError: + return 0This keeps the primary path unchanged while making misuse more obvious and less dangerous in terms of memory accounting.
Also applies to: 30-45
faststack.working-menus/faststack/qml/Components.qml (2)
49-55: Clamp zoom scale to avoid extreme values
onWheelmultiplies the current scale indefinitely, so long scrolls can make the image tiny or enormous and potentially destabilize panning. Clamping to a reasonable range improves UX and avoids numeric edge cases.// Wheel for zoom onWheel: function(wheel) { // A real implementation would be more complex, zooming // into the cursor position. - var scaleFactor = wheel.angleDelta.y > 0 ? 1.2 : 1 / 1.2; - scaleTransform.xScale *= scaleFactor; - scaleTransform.yScale *= scaleFactor; + var scaleFactor = wheel.angleDelta.y > 0 ? 1.2 : 1 / 1.2; + var newScale = Math.max(0.1, Math.min(8.0, scaleTransform.xScale * scaleFactor)); + scaleTransform.xScale = newScale; + scaleTransform.yScale = newScale; }Also applies to: 138-145
90-123: Drag‑outside detection mixes scene and local coordinates
mapToItem(null, mouse.x, mouse.y)returns scene coordinates, but you compare them with0..loupeView.width/height, which are in loupeView’s local space. If loupeView is ever inset inside the window, this can trigger drag‑and‑drop while the pointer is still visually inside the view.Consider either mapping into
loupeView’s space (mapToItem(loupeView, ...)) or comparing against the actual window/root item dimensions instead ofloupeView.width/height.faststack.working-menus/faststack/io/watcher.py (1)
72-74: Makeis_alive()return a strictbool
return self.observer and self.observer.is_alive()can return theObserverinstance orFalse, which doesn’t match the annotatedboolreturn type. Wrapping inbool()keeps the public API consistent.def is_alive(self) -> bool: """Checks if the watcher thread is alive.""" - return self.observer and self.observer.is_alive() + return bool(self.observer and self.observer.is_alive())faststack.working-menus/faststack/config.py (1)
58-67: Uselog.exceptioninsave()to retain tracebackOn write failures you currently log with
log.error, which drops the stack trace. Usinglog.exceptionhere would keep the same behavior while preserving useful debugging information. Based on static analysis hints.def save(self): """Saves the current configuration to the INI file.""" try: self.config_path.parent.mkdir(parents=True, exist_ok=True) with self.config_path.open("w") as f: self.config.write(f) log.info(f"Saved config to {self.config_path}") except IOError as e: - log.error(f"Failed to save config to {self.config_path}: {e}") + log.exception(f"Failed to save config to {self.config_path}")faststack/faststack/imaging/editor.py (2)
53-87: Tightenload_imageerror handling and avoid bareException+
load_imagecurrently catchesExceptionand prints to stdout. For a GUI app this is easy to miss and hides useful context. Narrowing the exception types (e.g., toOSError/Pillow errors) and logging via your standard logger (withlog.exception) would make failures diagnosable while still returningFalseto the caller. Based on static analysis hints.
69-80: Verify cached preview buffer layout when usingImage.frombytes
Image.frombytes("RGB", (cached_preview.width, cached_preview.height), bytes(cached_preview.buffer))assumes the buffer is tightly packed RGB888 (bytes_per_line == width * 3) with no row padding and matching channel order. IfDecodedImageever carries a differentQImageformat or padding, this will produce corrupt previews.Consider either asserting the invariant:
assert cached_preview.bytes_per_line == cached_preview.width * 3or passing the stride/format explicitly, e.g.:
self._preview_image = Image.frombytes( "RGB", (cached_preview.width, cached_preview.height), bytes(cached_preview.buffer), "raw", "RGB", cached_preview.bytes_per_line, 1, )faststack.working-menus/faststack/qml/Main.qml (1)
223-239: Wire&Open Folder...menu action to controller logicThe File →
&Open Folder...action has noonTriggeredhandler, so it currently does nothing. Consider calling into your existing directory-selection / default-directory logic (e.g., viauiState.open_directory_dialog()+controller.set_default_directory(...)or similar) so users can actually switch folders from the menu.faststack/faststack/app.py (2)
633-679: Minor style: remove unnecessaryfprefixes on constant status messagesThese calls don’t interpolate any variables:
self.update_status_message(f"Removed from batch") ... self.update_status_message(f"Removed from stack")You can simplify them to plain strings:
- self.update_status_message(f"Removed from batch") + self.update_status_message("Removed from batch") ... - self.update_status_message(f"Removed from stack") + self.update_status_message("Removed from stack")
1786-1816: Remove or use unusedb_diffin auto white balanceIn
auto_white_balance,b_diffis computed but never used:r_diff = r_mean - grey_target g_diff = g_mean - grey_target b_diff = b_mean - grey_target # unusedSince the subsequent math only uses
r_diffandg_diff, either logb_difffor diagnostics or drop the assignment to keep linters quiet:- r_diff = r_mean - grey_target - g_diff = g_mean - grey_target - b_diff = b_mean - grey_target + r_diff = r_mean - grey_target + g_diff = g_mean - grey_target + # b_diff could be logged if needed for diagnostics, but isn't used in the solve.faststack.working-menus/faststack/ui/provider.py.bak (1)
1-247: Clarify or remove.bakprovider module to avoid confusionThis backup file defines its own
ImageProviderandUIState, but it:
- Uses
configandself.ui_state(e.g., inset_theme) without importing/defining them here.- Diverges from the primary
faststack.ui.providerused by the app.If this module isn’t meant to be imported at runtime, consider removing it from the repo or ensuring your tooling (packaging, Ruff, etc.) ignores it. Otherwise, it should be brought in sync with the real provider and fixed up to avoid undefined-name issues.
faststack.working-menus/faststack/app.py (1)
643-679: Minor cleanups: constant f-strings and logging styleWithin
remove_from_batch_or_stack, you have constant f-strings and a warning with an f-string that doesn’t interpolate:self.update_status_message(f"Removed from batch") ... self.update_status_message(f"Removed from stack")These can be plain strings:
- self.update_status_message(f"Removed from batch") + self.update_status_message("Removed from batch") ... - self.update_status_message(f"Removed from stack") + self.update_status_message("Removed from stack")Similarly, where you log an error with an exception object (e.g., in
_delete_temp_file), usinglog.exception(...)instead oflog.error(...)will automatically attach the traceback and usually plays better with TRY400-style checks.faststack/faststack/ui/provider.py (2)
28-36: Handle unusedsize/requestedSizeparameters explicitly for clarity and linting.
requestImagemust acceptsizeandrequestedSizefor the QQuickImageProvider API but never uses them. To keep Ruff and future readers happy, consider renaming to_size/_requestedSizeor adding an inline comment to indicate they’re intentionally unused.def requestImage(self, id: str, _size: object, _requestedSize: object) -> QImage: ...
63-66: Logging style inexceptblock (optional).Static analysis suggests using
logging.exceptionhere to include a traceback. Since this is already a handled, non-fatal condition,log.erroris acceptable; switch tolog.exceptiononly if you want stack traces in this path.faststack.working-menus/patch (1)
295-317: Use the exposed property instead of the private_is_editingflag in_handle_editing_toggle.Currently:
is_editing = self.app_controller.app_state_model.is_editing if is_editing: self.app_controller.app_state_model.current_edited_image_data = ( self.app_controller.image_editor.get_preview_data() )Accessing the private
_is_editingfield viais_editingundermines the@Propertyencapsulation and can drift if the implementation changes. Prefer the public property:is_editing = self.app_controller.app_state_model.isEditing(or expose a small helper on AppStateModel if needed).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (55)
faststack.working-menus/ChangeLog.md(1 hunks)faststack.working-menus/LICENSE(1 hunks)faststack.working-menus/README.md(1 hunks)faststack.working-menus/faststack.egg-info/PKG-INFO(1 hunks)faststack.working-menus/faststack.egg-info/SOURCES.txt(1 hunks)faststack.working-menus/faststack.egg-info/dependency_links.txt(1 hunks)faststack.working-menus/faststack.egg-info/entry_points.txt(1 hunks)faststack.working-menus/faststack.egg-info/requires.txt(1 hunks)faststack.working-menus/faststack.egg-info/top_level.txt(1 hunks)faststack.working-menus/faststack/app.py(1 hunks)faststack.working-menus/faststack/benchmark_decode.py(1 hunks)faststack.working-menus/faststack/config.py(1 hunks)faststack.working-menus/faststack/imaging/cache.py(1 hunks)faststack.working-menus/faststack/imaging/jpeg.py(1 hunks)faststack.working-menus/faststack/imaging/prefetch.py(1 hunks)faststack.working-menus/faststack/io/executable_validator.py(1 hunks)faststack.working-menus/faststack/io/helicon.py(1 hunks)faststack.working-menus/faststack/io/indexer.py(1 hunks)faststack.working-menus/faststack/io/sidecar.py(1 hunks)faststack.working-menus/faststack/io/watcher.py(1 hunks)faststack.working-menus/faststack/logging_setup.py(1 hunks)faststack.working-menus/faststack/models.py(1 hunks)faststack.working-menus/faststack/qml/Components.qml(1 hunks)faststack.working-menus/faststack/qml/FilterDialog.qml(1 hunks)faststack.working-menus/faststack/qml/JumpToImageDialog.qml(1 hunks)faststack.working-menus/faststack/qml/Main.qml(1 hunks)faststack.working-menus/faststack/qml/SettingsDialog.qml(1 hunks)faststack.working-menus/faststack/tests/test_cache.py(1 hunks)faststack.working-menus/faststack/tests/test_executable_validator.py(1 hunks)faststack.working-menus/faststack/tests/test_pairing.py(1 hunks)faststack.working-menus/faststack/tests/test_sidecar.py(1 hunks)faststack.working-menus/faststack/ui/keystrokes.py(1 hunks)faststack.working-menus/faststack/ui/keystrokes.py.bak(1 hunks)faststack.working-menus/faststack/ui/provider.py(1 hunks)faststack.working-menus/faststack/ui/provider.py.bak(1 hunks)faststack.working-menus/patch(1 hunks)faststack.working-menus/pyproject.toml(1 hunks)faststack.working-menus/requirements.txt(1 hunks)faststack/ChangeLog.md(1 hunks)faststack/README.md(4 hunks)faststack/faststack.egg-info/PKG-INFO(2 hunks)faststack/faststack.egg-info/requires.txt(0 hunks)faststack/faststack/app.py(18 hunks)faststack/faststack/imaging/editor.py(1 hunks)faststack/faststack/imaging/jpeg.py(4 hunks)faststack/faststack/io/helicon.py(1 hunks)faststack/faststack/qml/Components.qml(2 hunks)faststack/faststack/qml/ImageEditorDialog.qml(1 hunks)faststack/faststack/qml/ImageEditorDialog.qml.new(1 hunks)faststack/faststack/qml/ImageEditorDialog.qml.old(1 hunks)faststack/faststack/qml/Main.qml(8 hunks)faststack/faststack/ui/keystrokes.py(1 hunks)faststack/faststack/ui/provider.py(3 hunks)faststack/pyproject.toml(1 hunks)faststack/requirements.txt(0 hunks)
💤 Files with no reviewable changes (2)
- faststack/faststack.egg-info/requires.txt
- faststack/requirements.txt
🧰 Additional context used
🧬 Code graph analysis (13)
faststack.working-menus/faststack/logging_setup.py (1)
faststack/faststack/config.py (2)
AppConfig(33-83)__init__(34-37)
faststack.working-menus/faststack/imaging/jpeg.py (1)
faststack/faststack/imaging/prefetch.py (1)
Prefetcher(129-487)
faststack.working-menus/faststack/io/helicon.py (2)
faststack.working-menus/faststack/io/executable_validator.py (1)
validate_executable_path(23-95)faststack.working-menus/faststack/config.py (1)
get(68-69)
faststack.working-menus/faststack/benchmark_decode.py (1)
faststack.working-menus/faststack/imaging/jpeg.py (1)
decode_jpeg_resized(99-149)
faststack.working-menus/faststack/io/indexer.py (2)
faststack.working-menus/faststack/models.py (1)
ImageFile(8-12)faststack/faststack/tests/test_pairing.py (2)
test_find_images(36-50)mock_image_dir(13-34)
faststack.working-menus/faststack/imaging/cache.py (2)
faststack.working-menus/faststack/models.py (1)
DecodedImage(35-44)faststack/faststack/tests/test_cache.py (4)
test_cache_add_items(21-29)test_cache_init(15-19)test_cache_update_item(49-61)MockItem(7-13)
faststack/faststack/imaging/editor.py (2)
faststack.working-menus/faststack/models.py (1)
DecodedImage(35-44)faststack/faststack/app.py (2)
get_preview_data(1643-1645)set_crop_box(1660-1665)
faststack.working-menus/faststack/io/sidecar.py (3)
faststack/faststack/app.py (1)
load(274-287)faststack/faststack/models.py (2)
Sidecar(27-32)EntryMetadata(15-23)faststack/faststack/tests/test_sidecar.py (3)
test_sidecar_save(47-65)test_sidecar_load_existing(28-45)test_sidecar_load_non_existent(20-26)
faststack.working-menus/faststack/io/executable_validator.py (1)
faststack/faststack/tests/test_executable_validator.py (7)
test_suspicious_path_with_traversal(51-68)test_wrong_executable_name_for_type(112-131)test_is_executable_windows(87-96)test_non_exe_file(71-84)test_empty_path(14-18)test_valid_photoshop_path(28-48)test_is_subpath(99-109)
faststack.working-menus/faststack/models.py (3)
faststack/faststack/models.py (4)
EntryMetadata(15-23)ImageFile(8-12)Sidecar(27-32)DecodedImage(35-44)faststack/faststack/io/sidecar.py (2)
SidecarManager(13-99)get_metadata(94-96)faststack/faststack/tests/test_sidecar.py (1)
test_sidecar_get_metadata_creates_new(67-74)
faststack.working-menus/faststack/config.py (1)
faststack.working-menus/faststack/logging_setup.py (1)
get_app_data_dir(8-13)
faststack.working-menus/faststack/imaging/prefetch.py (4)
faststack.working-menus/faststack/models.py (2)
ImageFile(8-12)DecodedImage(35-44)faststack.working-menus/faststack/imaging/jpeg.py (2)
decode_jpeg_rgb(22-40)decode_jpeg_resized(99-149)faststack.working-menus/faststack/config.py (1)
get(68-69)faststack/faststack/app.py (1)
get_display_info(188-191)
faststack/faststack/app.py (4)
faststack.working-menus/faststack/ui/provider.py (3)
ImageProvider(21-66)UIState(69-342)launch_helicon(258-259)faststack.working-menus/faststack/io/sidecar.py (1)
save(62-91)faststack.working-menus/faststack/imaging/prefetch.py (1)
cancel_all(474-481)faststack.working-menus/faststack/models.py (1)
DecodedImage(35-44)
🪛 LanguageTool
faststack.working-menus/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 1.0 - November 21, 2025 # By Alan Rockefeller Ultra-fast, cach...
(MISSING_COMMA_AFTER_YEAR)
[grammar] ~12-~12: Ensure spelling is correct
Context: ...0ms next/previous image switching, high peformance decoding via PyTurboJPEG. - **Zoom & ...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
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 1.2 - November 22, 2025 # By Alan Rockefeller Ultra-fast, cach...
(MISSING_COMMA_AFTER_YEAR)
[grammar] ~12-~12: Ensure spelling is correct
Context: ...0ms next/previous image switching, high-peformance decoding via PyTurboJPEG. - **Zoom & ...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
faststack.working-menus/ChangeLog.md
[style] ~103-~103: Consider using a different verb for a more formal wording.
Context: ...he JPEG scaling factor calculation. - Fixed an issue where panning the image was no...
(FIX_RESOLVE)
[grammar] ~118-~118: Use a hyphen to join words.
Context: ...sion 0.4 ### Todo Make it use the full res image when zooming in When multiple ...
(QB_NEW_EN_HYPHEN)
🪛 Ruff (0.14.5)
faststack.working-menus/faststack/imaging/jpeg.py
30-30: Redundant exception object included in logging.exception call
(TRY401)
39-39: Redundant exception object included in logging.exception call
(TRY401)
65-65: Consider moving this statement to an else block
(TRY300)
67-67: Redundant exception object included in logging.exception call
(TRY401)
76-76: Redundant exception object included in logging.exception call
(TRY401)
130-130: Redundant exception object included in logging.exception call
(TRY401)
148-148: Redundant exception object included in logging.exception call
(TRY401)
faststack.working-menus/faststack/io/helicon.py
70-70: Redundant exception object included in logging.exception call
(TRY401)
77-77: subprocess call: check for execution of untrusted input
(S603)
85-85: Consider moving this statement to an else block
(TRY300)
87-87: Redundant exception object included in logging.exception call
(TRY401)
90-90: Redundant exception object included in logging.exception call
(TRY401)
faststack.working-menus/faststack/io/indexer.py
39-39: Local variable e is assigned to but never used
Remove assignment to unused variable e
(F841)
65-65: Unused function argument: jpg_path
(ARG001)
faststack.working-menus/faststack/tests/test_sidecar.py
14-14: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
faststack.working-menus/faststack/tests/test_pairing.py
55-55: Multiple statements on one line (semicolon)
(E702)
58-58: Multiple statements on one line (semicolon)
(E702)
58-58: Multiple statements on one line (semicolon)
(E702)
63-63: Multiple statements on one line (semicolon)
(E702)
63-63: Multiple statements on one line (semicolon)
(E702)
68-68: Multiple statements on one line (semicolon)
(E702)
68-68: Multiple statements on one line (semicolon)
(E702)
69-69: Multiple statements on one line (semicolon)
(E702)
69-69: Multiple statements on one line (semicolon)
(E702)
faststack.working-menus/faststack/tests/test_executable_validator.py
40-40: Unused lambda argument: self
(ARG005)
62-62: Unused lambda argument: self
(ARG005)
66-66: Unpacked variable error is never used
Prefix it with an underscore or any other dummy variable pattern
(RUF059)
123-123: Unused lambda argument: self
(ARG005)
127-127: Unpacked variable error is never used
Prefix it with an underscore or any other dummy variable pattern
(RUF059)
faststack/faststack/imaging/editor.py
81-81: Consider moving this statement to an else block
(TRY300)
82-82: Do not catch blind exception: Exception
(BLE001)
285-285: Do not catch blind exception: Exception
(BLE001)
290-290: Consider moving this statement to an else block
(TRY300)
291-291: Do not catch blind exception: Exception
(BLE001)
faststack.working-menus/faststack/io/sidecar.py
58-58: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
88-88: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
faststack.working-menus/faststack/app.py
43-43: Redefinition of unused ImageProvider from line 16
Remove definition: ImageProvider
(F811)
364-364: Local variable e is assigned to but never used
Remove assignment to unused variable e
(F841)
644-644: f-string without any placeholders
Remove extraneous f prefix
(F541)
678-678: f-string without any placeholders
Remove extraneous f prefix
(F541)
913-913: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
1124-1124: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
1297-1297: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
1357-1357: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
1366-1366: subprocess call: check for execution of untrusted input
(S603)
1522-1522: Loop control variable i not used within loop body
Rename unused i to _i
(B007)
1524-1524: Local variable count_in_batch is assigned to but never used
Remove assignment to unused variable count_in_batch
(F841)
1525-1525: Local variable pos_in_batch is assigned to but never used
Remove assignment to unused variable pos_in_batch
(F841)
faststack.working-menus/faststack/io/executable_validator.py
110-110: Consider moving this statement to an else block
(TRY300)
faststack.working-menus/faststack/config.py
66-66: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
faststack.working-menus/faststack/imaging/prefetch.py
455-455: Consider moving this statement to an else block
(TRY300)
faststack/faststack/app.py
137-137: Undefined name Tuple
(F821)
655-655: f-string without any placeholders
Remove extraneous f prefix
(F541)
689-689: f-string without any placeholders
Remove extraneous f prefix
(F541)
1815-1815: Local variable b_diff is assigned to but never used
Remove assignment to unused variable b_diff
(F841)
faststack.working-menus/faststack/ui/provider.py
28-28: Unused method argument: size
(ARG002)
28-28: Unused method argument: requestedSize
(ARG002)
64-64: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
🔇 Additional comments (19)
faststack/faststack/ui/keystrokes.py (1)
29-29: Key mappings are logical, well-organized, and properly implemented.The updated keyboard shortcuts are intuitive and properly grouped by functionality. All four new controller methods are present and defined:
toggle_stack_membership(app.py:782)toggle_batch_membership(app.py:708)edit_in_photoshop(app.py:1394)quick_auto_white_balance(app.py:1732)Note that
Key_Sis now used in two contexts: plainSmaps totoggle_stack_membership(line 29) whileCtrl+Smaps totoggle_stacked(line 58). While both are stack-related actions, they trigger different methods. This is acceptable but worth keeping in mind for user experience.faststack.working-menus/LICENSE (1)
1-25: LGTM!Standard MIT License with proper copyright notice and all required clauses.
faststack.working-menus/faststack/benchmark_decode.py (1)
11-20: LGTM!The benchmark correctly mimics the real code path using mmap for file reading and measures decode performance over multiple iterations. The timing methodology is sound.
faststack.working-menus/requirements.txt (1)
1-9: <!-- [scratchpad_start]
[task review_dependencies verification_1/1]
Verify consistency of dependency specifications across faststack packages.[observations]
- faststack/pyproject.toml uses range specs: PySide6>=6.0,<7.0, PyTurboJPEG>=1.8,<2.0, numpy>=2.0,<3.0, cachetools>=5.0,<6.0, watchdog>=4.0,<5.0, Pillow>=10.0,<11.0, pytest>=8.0,<9.0
- faststack.working-menus/pyproject.toml is IDENTICAL to main faststack (same range specs)
- faststack.working-menus/requirements.txt uses wildcard pins: ==6.10., ==1., ==2., ==5., ==4., ==0.12., ==10., ==6., ==8.*
- requirements.txt includes typer==0.12.* which is NOT in either pyproject.toml
- requirements.txt includes pyinstaller==6.* which is NOT in either pyproject.toml
- All other packages match across files but use different version specification styles
[analysis]
- Original review correctly identified typer and version spec inconsistencies
- BUT missed pyinstaller==6.* which is only in requirements.txt
- The two pyproject.toml files are aligned, but requirements.txt diverges significantly
- This suggests requirements.txt may be a development/build environment file, but this is not documented
- The wildcard versions (e.g., ==6.10.*) are actually MORE restrictive than the range specs (e.g., >=6.0,<7.0)
[done]
Verification complete. Inconsistencies confirmed with details.
[scratchpad_end]
-->Clarify the role of requirements.txt and address dependency inconsistencies.
Verification confirms the version specification differences. However, requirements.txt contains two additional dependencies not found in any pyproject.toml:
typer==0.12.*pyinstaller==6.*The pyproject.toml files in both
faststackandfaststack.working-menusare identical and use broader version ranges (e.g.,PySide6>=6.0,<7.0), while requirements.txt uses restrictive wildcard pins (e.g.,PySide6==6.10.*).Clarify whether requirements.txt is meant for development/build-only dependencies. If so, document this separation and ensure pyproject.toml declares all production dependencies. If requirements.txt should align with pyproject.toml, consider whether
typerandpyinstallershould be added to pyproject.toml or removed from requirements.txt.faststack/faststack/imaging/jpeg.py (1)
19-28: Enhanced initialization improves robustness.The addition of a try/except block around
TurboJPEG()instantiation is excellent defensive programming. This ensures that even if PyTurboJPEG is installed but fails to initialize (e.g., missing libjpeg-turbo library), the code gracefully falls back to Pillow.faststack.working-menus/ChangeLog.md (1)
1-172: Well-documented change history.The changelog provides comprehensive documentation of the project's evolution with clear categorization of features, fixes, and improvements. This will be valuable for users and contributors.
faststack.working-menus/faststack/models.py (1)
1-44: LGTM! Well-structured data models.The dataclasses are cleanly defined with appropriate type hints and defaults. The
__sizeof__implementation inDecodedImagecorrectly returns the buffer size for memory-aware caching.faststack/faststack/qml/ImageEditorDialog.qml.new (1)
1-1: Clarify the.qml.newfile extension.The file has a
.qml.newextension, which suggests it may be a temporary or work-in-progress file. Ensure this file is renamed to.qmlbefore merging, or clarify the intent of this naming.faststack/faststack/io/helicon.py (1)
78-86: LGTM! Excellent security hardening for subprocess execution.The subprocess call correctly uses
shell=False,close_fds=True, and redirects all standard streams toDEVNULL, preventing shell injection and minimizing attack surface.faststack.working-menus/faststack/tests/test_cache.py (1)
1-61: LGTM!The test coverage for
ByteLRUCacheis comprehensive and well-structured. The tests correctly verify initialization, item addition, eviction behavior (LRU policy), and size tracking during updates. TheMockItemclass with__sizeof__provides a clean way to test size-aware caching.faststack/faststack/qml/Components.qml (1)
30-31: LGTM! Performance optimization for slider interactions.Disabling
smoothandmipmapduring slider interactions is a good optimization to maintain UI responsiveness. The null-safety check onuiStateis correct.faststack.working-menus/faststack/ui/keystrokes.py (1)
1-109: LGTM! Well-structured keyboard shortcut handler.The
Keybinderclass provides a robust keyboard shortcut system with:
- Clear separation of single-key and modifier-key bindings
- Proper fallback chain (QML → controller → warning)
- Text-based fallback for keyboard layout variations (lines 92-107)
- Good debug logging
The implementation handles edge cases well, particularly the text-based fallback for bracket/brace keys that may have different keycodes on different keyboard layouts.
faststack.working-menus/faststack/qml/SettingsDialog.qml (1)
18-25: Settings dialog wiring and validation look solidNice job on the settings dialog: visibility drives
dialog_opened/dialog_closedand cache usage polling, cache size is range‑checked and normalized, and path fields provide immediate existence feedback. Applying changes only ononAcceptedkeeps Cancel semantics clean. No issues from my side here.Also applies to: 39-46, 99-121, 164-171
faststack.working-menus/faststack/io/executable_validator.py (1)
23-95: Executable path validation logic looks solidThe sequence of checks in
validate_executable_path(resolution, existence/file, executability, app‑type name checks, safe‑directory checks, and suspicious‑path warnings) is clear and matches the scenarios covered intest_executable_validator.py. I don’t see correctness issues in this module.faststack.working-menus/faststack/imaging/prefetch.py (1)
30-455: Prefetcher + ICC/saturation pipeline looks robustThe ICC transform caching, monitor profile caching, and prefetch scheduling (adaptive radius + directional bias + generation checks) are all wired in a sane way; mmap usage plus the defensive
generationre-check beforecache_putshould prevent stale frames from leaking into the cache. I don’t see correctness or threading issues in this module as written.faststack.working-menus/faststack/app.py (1)
1307-1399: Photoshop launch flow is reasonably hardened against injectionRuff flags the
subprocess.Popencall (S603), but in this context you already:
- Validate
photoshop_exewithvalidate_executable_path(...)(existence, executability, safe dirs).- Parse
photoshop_argsviashlex.split(...).- Pass
shell=Falseand buildcommandas a list of arguments.Given those safeguards, this usage looks acceptable from a security perspective for a desktop app. I’d keep the current pattern; just ensure
validate_executable_pathstays strict about allowed locations/binaries.faststack.working-menus/patch (3)
239-287: ImageEditorDialog sliders and crop UI look consistent with the backend API.The slider model keys (
brightness,contrast,saturation,white_balance_by,white_balance_mg) match the keys expected byImageEditor.set_edit_param, and the values are correctly normalized back to[-1.0, 1.0]viaslider.value / 100.0. Crop toggle and aspect ratio dropdown also align with the new AppStateModel properties.
380-404: Editor parameter wiring from QML → AppController → AppStateModel is coherent.
set_edit_parameterupdates the ImageEditor model, regenerates the PNG preview, marksedits_pending, and then mirrors the value back into AppStateModel (brightness,contrast, etc.), which QML binds to. This keeps previews and sliders in sync and avoids drift between UI state and the underlying editor.
361-379: <!-- [scratchpad_start]
[task verification_1/1]
[observations]
- First search for
app_controller.pyreturned no results- File does not exist in repository root or subdirectories
- Review comment patch file is:
faststack.working-menus/patch- Code snippet contains methods:
get_aspect_ratiosandset_edit_parameterwith@Slotdecorators- This may be a new file being created by the patch, or the file path in the review is incorrect
[analysis]
- Cannot verify imports if the file doesn't exist yet (patch may create it)
- Need to search for the actual file containing these methods or find similar patterns
- Should search for
get_aspect_ratiosandset_edit_parametermethods to locate the actual file being modified[pending]
- Search for methods
get_aspect_ratiosorset_edit_parameterin codebase- Locate the actual Python file being patched
- Verify import status once file is located
[actions]
- Run
rgsearch for "get_aspect_ratios" to find where this code actually exists- If found, check imports in that file
| [console_scripts] | ||
| faststack = faststack.app:main |
There was a problem hiding this comment.
🧩 Analysis chain
Verify entry point function consistency.
The AI summary indicates that pyproject.toml maps the console script to faststack.app:cli, but this file maps it to faststack.app:main. Ensure both files reference the same entry point function to avoid runtime errors.
Run the following script to check which entry point function exists and verify consistency:
🏁 Script executed:
#!/bin/bash
# Check for main() and cli() functions in faststack/app.py
rg -nP '^\s*def\s+(main|cli)\s*\(' --type=py -g '**/app.py'
# Check pyproject.toml for console_scripts configuration
fd -t f 'pyproject.toml' --exec rg -A5 'console_scripts'Length of output: 281
🏁 Script executed:
#!/bin/bash
# Find and display pyproject.toml console_scripts configuration
fd -t f 'pyproject.toml' --exec cat {} \; | grep -A 10 'console_scripts' || echo "console_scripts section not found in pyproject.toml"Length of output: 51
🏁 Script executed:
#!/bin/bash
# Search for setup.py, setup.cfg, and check full pyproject.toml content
echo "=== setup.py files ==="
fd -t f 'setup.py' --exec cat {} \;
echo -e "\n=== setup.cfg files ==="
fd -t f 'setup.cfg' --exec cat {} \;
echo -e "\n=== Full pyproject.toml content ==="
fd -t f 'pyproject.toml' --exec cat {} \;Length of output: 4886
Delete stale egg-info directory and rebuild to sync entry point with pyproject.toml.
Verification confirms the mismatch: pyproject.toml defines the entry point as faststack.app:cli, but the auto-generated faststack.egg-info/entry_points.txt contains faststack.app:main. This causes the installed package to use the wrong entry point at runtime.
The egg-info directory must be regenerated from pyproject.toml during build. Delete faststack.egg-info/ and run a clean build to resolve the inconsistency.
🤖 Prompt for AI Agents
In faststack.working-menus/faststack.egg-info/entry_points.txt around lines 1 to
2, the auto-generated entry point points to faststack.app:main while
pyproject.toml declares faststack.app:cli; delete the stale faststack.egg-info/
directory and perform a clean rebuild (remove build/ and dist/ if present, then
run the project build/install command you use — e.g. python -m build or pip
install .) so egg-info is regenerated from pyproject.toml and the installed
package will use faststack.app:cli.
| from faststack.ui.provider import ImageProvider, UIState | ||
| from PySide6.QtGui import QDrag, QPixmap | ||
| from PySide6.QtCore import ( | ||
| QUrl, | ||
| QTimer, | ||
| QObject, | ||
| QEvent, | ||
| Signal, | ||
| Slot, | ||
| QMimeData, | ||
| Qt, | ||
| QPoint | ||
| ) | ||
| from PySide6.QtWidgets import QApplication, QFileDialog, QMessageBox | ||
| from PySide6.QtQml import QQmlApplicationEngine | ||
|
|
||
| # ⬇️ these are the ones that went missing | ||
| from faststack.config import config | ||
| from faststack.logging_setup import setup_logging | ||
| from faststack.models import ImageFile, DecodedImage, EntryMetadata | ||
| from faststack.io.indexer import find_images | ||
| from faststack.io.sidecar import SidecarManager | ||
| from faststack.io.watcher import Watcher | ||
| from faststack.io.helicon import launch_helicon_focus | ||
| from faststack.io.executable_validator import validate_executable_path | ||
| from faststack.imaging.cache import ByteLRUCache, get_decoded_image_size | ||
| from faststack.imaging.prefetch import Prefetcher, clear_icc_caches | ||
| from faststack.ui.provider import ImageProvider | ||
| from faststack.ui.keystrokes import Keybinder | ||
|
|
There was a problem hiding this comment.
Remove duplicate ImageProvider import
You import ImageProvider twice:
from faststack.ui.provider import ImageProvider, UIState # line 16
...
from faststack.ui.provider import ImageProvider # line 43The second import is redundant and triggers Ruff’s F811 (“redefinition of unused ImageProvider”). It’s safe to drop the second line:
-from faststack.ui.provider import ImageProvider🧰 Tools
🪛 Ruff (0.14.5)
43-43: Redefinition of unused ImageProvider from line 16
Remove definition: ImageProvider
(F811)
🤖 Prompt for AI Agents
In faststack.working-menus/faststack/app.py around lines 16 to 45 you have a
duplicate import of ImageProvider (first at line ~16 with UIState and again
later), which causes a redefinition lint error; remove the second "from
faststack.ui.provider import ImageProvider" line (or merge if any other names
are present) so ImageProvider is only imported once and keep the original import
that includes UIState.
|
|
||
| print(f"TurboJPEG available: {TURBO_AVAILABLE}") | ||
|
|
||
| test_image = Path(r"C:\Users\alanr\Pictures\Lightroom\2025\2025-11-14\20251114-PB140001-2.JPG") |
There was a problem hiding this comment.
Remove hardcoded user-specific path.
The hardcoded path C:\Users\alanr\Pictures\... will fail for other users. Make the path configurable via command-line argument or environment variable.
Apply this diff to make the path configurable:
+import sys
import mmap
import time
from pathlib import Path
from faststack.imaging.jpeg import decode_jpeg_resized, TURBO_AVAILABLE
print(f"TurboJPEG available: {TURBO_AVAILABLE}")
-test_image = Path(r"C:\Users\alanr\Pictures\Lightroom\2025\2025-11-14\20251114-PB140001-2.JPG")
+if len(sys.argv) < 2:
+ print("Usage: python benchmark_decode.py <path_to_jpeg>")
+ sys.exit(1)
+test_image = Path(sys.argv[1])
+
+if not test_image.exists():
+ print(f"Error: Image file not found: {test_image}")
+ sys.exit(1)🤖 Prompt for AI Agents
faststack.working-menus/faststack/benchmark_decode.py around line 8: the
test_image variable is set to a hardcoded, user-specific absolute path which
will break for other users; replace it by reading the image path from a
command-line argument (use argparse) with a sensible default or fallback to an
environment variable (e.g., TEST_IMAGE_PATH) if the argument is not provided;
validate that the path exists and raise a clear error if not, and remove the
hardcoded literal so callers can run the script with --image /path/to/image or
by setting TEST_IMAGE_PATH.
| try: | ||
| from turbojpeg import TurboJPEG, TJPF_RGB | ||
| jpeg_decoder = TurboJPEG() | ||
| TURBO_AVAILABLE = True | ||
| log.info("PyTurboJPEG is available. Using for JPEG decoding.") | ||
| except ImportError: | ||
| jpeg_decoder = None | ||
| TURBO_AVAILABLE = False | ||
| log.warning("PyTurboJPEG not found. Falling back to Pillow for JPEG decoding.") |
There was a problem hiding this comment.
Add initialization error handling for TurboJPEG.
This version lacks the enhanced initialization error handling present in faststack/faststack/imaging/jpeg.py (lines 19-28). The TurboJPEG() constructor can fail even when the module imports successfully (e.g., if the underlying libjpeg-turbo library is missing).
Apply this diff to match the more robust initialization:
try:
from turbojpeg import TurboJPEG, TJPF_RGB
- jpeg_decoder = TurboJPEG()
- TURBO_AVAILABLE = True
- log.info("PyTurboJPEG is available. Using for JPEG decoding.")
except ImportError:
jpeg_decoder = None
TURBO_AVAILABLE = False
log.warning("PyTurboJPEG not found. Falling back to Pillow for JPEG decoding.")
+else:
+ try:
+ jpeg_decoder = TurboJPEG()
+ except Exception:
+ jpeg_decoder = None
+ TURBO_AVAILABLE = False
+ log.exception("PyTurboJPEG initialization failed. Falling back to Pillow.")
+ else:
+ TURBO_AVAILABLE = True
+ log.info("PyTurboJPEG is available. Using it for JPEG decoding.")🤖 Prompt for AI Agents
In faststack.working-menus/faststack/imaging/jpeg.py around lines 12 to 20, the
import block assumes TurboJPEG() will always initialize successfully; change it
to first try importing TurboJPEG and then attempt to construct TurboJPEG() in a
nested try/except that catches any Exception from the constructor, logging an
error with the exception details, setting jpeg_decoder = None and
TURBO_AVAILABLE = False on failure; keep the original ImportError handling for
missing module but ensure both import and init failures log distinct messages
and disable turbo fallback when either step fails.
| if not raw_files: | ||
| log.warning("No RAW files selected to open in Helicon Focus.") | ||
| return False, None | ||
|
|
||
| try: | ||
| with tempfile.NamedTemporaryFile("w", delete=False, suffix=".txt", encoding='utf-8') as tmp: | ||
| for f in raw_files: | ||
| # Ensure file path is resolved and exists | ||
| if not f.exists(): | ||
| log.warning(f"RAW file does not exist, skipping: {f}") | ||
| continue | ||
| tmp.write(f"{f.resolve()}\n") | ||
| tmp_path = Path(tmp.name) | ||
|
|
||
| log.info(f"Temporary file for Helicon Focus: {tmp_path}") | ||
| log.info(f"Input files: {[str(f) for f in raw_files]}") |
There was a problem hiding this comment.
Handle the case where all provided RAW files are missing and avoid launching Helicon with an empty list
Inside the temp file creation block you skip non‑existent raw_files entries, but you don’t check whether any valid paths were actually written. If the input list contains only missing files, Helicon is launched with an empty list, and an unnecessary temp file is left behind.
Consider tracking whether any files were written and short‑circuiting if not, e.g.:
- try:
- with tempfile.NamedTemporaryFile("w", delete=False, suffix=".txt", encoding='utf-8') as tmp:
- for f in raw_files:
- # Ensure file path is resolved and exists
- if not f.exists():
- log.warning(f"RAW file does not exist, skipping: {f}")
- continue
- tmp.write(f"{f.resolve()}\n")
- tmp_path = Path(tmp.name)
+ try:
+ wrote_any = False
+ with tempfile.NamedTemporaryFile("w", delete=False, suffix=".txt", encoding="utf-8") as tmp:
+ for f in raw_files:
+ if not f.exists():
+ log.warning("RAW file does not exist, skipping: %s", f)
+ continue
+ tmp.write(f"{f.resolve()}\n")
+ wrote_any = True
+ tmp_path = Path(tmp.name)
+
+ if not wrote_any:
+ log.warning("No existing RAW files found; not launching Helicon Focus.")
+ if tmp_path.exists():
+ tmp_path.unlink()
+ return False, NoneThis avoids confusing behavior and cleans up the unused temp file.
Also applies to: 73-86
🤖 Prompt for AI Agents
In faststack.working-menus/faststack/io/helicon.py around lines 41 to 56 (and
apply same fix to the similar block at lines 73-86), you currently skip
non-existent RAW files but don’t check whether any valid paths were written,
which can launch Helicon with an empty list and leave a temp file; modify the
logic to track whether at least one path was written to the temp file (e.g.,
increment a counter or append to a local list while iterating), and if none were
written, delete the temporary file (or ensure it is not left behind), log a
warning about no valid RAW files, and return False, None without launching
Helicon; apply the identical guard-and-cleanup change to the second block at
lines 73-86.
| FilterDialog { | ||
| id: filterDialog | ||
| onAccepted: { | ||
| controller.apply_filter(filterString) | ||
| if (uiState) uiState.applyFilter(filterString) | ||
| } |
There was a problem hiding this comment.
FilterDialog.onAccepted calls an undefined uiState.applyFilter slot.
FilterDialog invokes:
if (uiState) uiState.applyFilter(filterString)but UIState in faststack/faststack/ui/provider.py exposes filterString as a property only; there is no corresponding applyFilter slot or method. This will throw at runtime when the dialog is accepted.
Either:
- Implement a
@Slot(str)applyFiltermethod onUIStatethat forwards to the controller, or - Route the call directly to whatever controller method applies the filter (e.g.
controller.apply_filter(filterString)) and drop theuiState.applyFiltercall.
🤖 Prompt for AI Agents
In faststack/faststack/qml/Main.qml around lines 842 to 846,
FilterDialog.onAccepted calls uiState.applyFilter(filterString) but UIState
(faststack/faststack/ui/provider.py) has no applyFilter slot; this will error at
runtime. Fix by either (A) adding a @Slot(str) applyFilter(self, s) method to
UIState that forwards the string to the existing controller method that applies
filters, or (B) change Main.qml to call the controller method directly (e.g.
controller.apply_filter(filterString)) and remove the uiState.applyFilter call;
ensure QML bindings and imports still expose whichever object you choose to call
and update any tests or usages accordingly.
| [project] | ||
| name = "faststack" | ||
| version = "1.0" | ||
| version = "1.2" |
There was a problem hiding this comment.
🧩 Analysis chain
Version mismatch between packaging files.
The version is set to "1.2" here, but faststack/faststack.egg-info/PKG-INFO shows version "1.1". This inconsistency suggests the package metadata is out of sync with the project definition.
Run the following script to verify the version consistency across packaging files:
🏁 Script executed:
#!/bin/bash
# Description: Check version consistency across packaging files
echo "=== Version in pyproject.toml files ==="
fd -t f "pyproject.toml" --exec grep -H "version"
echo -e "\n=== Version in PKG-INFO files ==="
fd -t f "PKG-INFO" --exec grep -H "^Version:"
echo -e "\n=== Version in setup.py files (if any) ==="
fd -t f "setup.py" --exec grep -H "version"Length of output: 338
Confirmed: Version mismatch in faststack package metadata.
The script output confirms the inconsistency. The faststack/pyproject.toml declares version "1.2", but faststack/faststack.egg-info/PKG-INFO shows version "1.1". This indicates the egg-info metadata is out of sync and needs to be regenerated to match the project's declared version.
🤖 Prompt for AI Agents
In faststack/pyproject.toml around line 8 the declared version is "1.2" but the
egg-info metadata (faststack/faststack.egg-info/PKG-INFO) still reports "1.1";
remove the stale egg-info directory and regenerate package metadata so they
match: update the pyproject version if it was wrong, then delete
faststack/faststack.egg-info and run your project build/install command (e.g.,
python -m build or python -m pip install -e .) to recreate the egg-info with the
correct version.
1 participant
Summary by CodeRabbit
Release Notes
New Features
UI/UX Improvements
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.