Release version 1.6.4#95
Merged
Merged
Conversation
…e is active, which matches the new README guidance that crop must be applied or cancelled before saving/editing. Crop mode is now allowed while the compact editor is open, but still blocked when the expanded editor is open. That is an intentional behavior change and the guard looks localized. EXIF brief cache keys now include both current and previous image source keys so the “distance from previous photo” is order-sensitive. That is the right direction for filtered/sorted views, because the displayed previous image matters. Rotation now transforms an existing normalized crop box when 90-degree rotation changes. The transform logic looks plausible for normalized 0–1000 coordinates and should fix the “rotate already-cropped image” class of bugs. np.ascontiguousarray(arr_u8) before QImage serialization is a solid correctness fix for rotated/non-contiguous NumPy views. Highlight recovery and exposure/whites slider scaling are real visual-output changes, not cleanup. They appear intentional based on the changelog and README.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughVersion 1.6.4 adds auto-vibrance adjustment to the auto-levels pipeline, displays inter-image GPS distances in the status bar, improves crop geometry stability across 90° rotations, enhances trackpad crop interaction reliability via grace-timer deferral, refactors compact editor keyboard handling with slider scaling, updates highlight-recovery math and color-space LUTs, optimizes image provider robustness and prefetch targeting, and includes comprehensive documentation and tooling improvements. Changesv1.6.4 Release and Features
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
… the pending release and falls through to normal hit-testing; right-button presses only force
a new crop when entering crop mode, so active crop resize/move hit-tests run normally.
- faststack/imaging/editor.py:846: _apply_edits now has update_highlight_state; auto-vibrance analysis disables it so downsampled scratch renders do not
overwrite live clipping telemetry.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
faststack/imaging/metadata.py (1)
173-211: ⚡ Quick winConsolidate duplicated rational-to-float conversion logic.
_exif_rational_to_floatis functionally identical to the existing_exif_rational_to_seconds(lines 105-143), differing only in the function name and log messages. Consider extracting a shared helper to eliminate duplication and ensure consistent behavior.♻️ Suggested consolidation
+def _exif_rational_to_float(x: Any) -> Optional[float]: + """Convert EXIF rational-ish values to float.""" + return _exif_rational_to_seconds(x) + -def _exif_rational_to_float(x: Any) -> Optional[float]: - """Convert EXIF rational-ish values to float.""" - if x is None: - return None - if hasattr(x, "numerator") and hasattr(x, "denominator"): - try: - n, d = int(x.numerator), int(x.denominator) - if d != 0: - return float(Fraction(n, d)) - except Exception as e: - log.debug( - "_exif_rational_to_float failed for rational object %r (%s): %s", - x, - type(x).__name__, - e, - ) - if isinstance(x, (tuple, list)) and len(x) == 2: - try: - n, d = int(x[0]), int(x[1]) - if d != 0: - return float(Fraction(n, d)) - except Exception as e: - log.debug( - "_exif_rational_to_float failed for tuple/list %r (%s): %s", - x, - type(x).__name__, - e, - ) - try: - return float(x) - except Exception as e: - if x is not None: - log.debug( - "_exif_rational_to_float failed for value %r (%s): %s", - x, - type(x).__name__, - e, - ) - return NoneAlternatively, rename
_exif_rational_to_secondsto_exif_rational_to_floatand update the one caller at line 148.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@faststack/imaging/metadata.py` around lines 173 - 211, The two functions _exif_rational_to_float and _exif_rational_to_seconds contain duplicate conversion and logging logic; extract a single shared helper (e.g., _parse_exif_rational or _exif_rational_to_number) that performs the numerator/denominator and tuple/list handling and final float() conversion and logging, then have both _exif_rational_to_float and _exif_rational_to_seconds delegate to that helper (or have one thin wrapper if different semantics are needed), ensuring both use the same logging message/format to eliminate duplication and keep behavior consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ChangeLog.md`:
- Line 5: Update the release date for version 1.6.4 so it is consistent across
docs: change the date in the ChangeLog entry string "## 1.6.4 (2026-06-06)" to
match the README banner date "June 7, 2026" (and the PR creation timestamp
2026-06-07T17:20:40Z) so all references to the 1.6.4 release use the same
calendar date.
In `@faststack/app.py`:
- Around line 2576-2586: The reported vibrance delta can differ from the applied
value because base_vibrance is clamped; update the logic after computing
base_vibrance so the effective change is computed and used for messaging or
saving. Specifically, after calling self.image_editor.analyze_auto_vibrance (and
after calculating base_vibrance), compute effective_delta = base_vibrance -
current_vibrance (or set auto_vibrance_delta = base_vibrance - current_vibrance)
and use that effective_delta when generating status text or persisting changes
so messages reflect the clamped result rather than the raw analysis.
- Around line 3016-3018: The crop-save guard (the if self.ui_state.isCropping
... update_status_message ... return) must be moved out of the ad-hoc save path
and into the centralized save-preparation function
_prepare_current_session_save_request() so saving is consistently blocked while
a crop is active; update _prepare_current_session_save_request() to check
self.ui_state.isCropping, call update_status_message("Apply or cancel the crop
before saving") and return a failure/abort result instead of silently
auto-cancelling, and remove any duplicate auto-cancel logic so callers like
start_drag_current_image and prepare_for_app_close rely on the centralized
behavior.
In `@faststack/qml/CompactEditorWindow.qml`:
- Around line 224-226: The E-key branch currently sets
compactEditor.uiStateRef.isEditorOpen = false directly when event.key ===
Qt.Key_E, bypassing the guarded close flow used by Escape/close-button; change
this branch to invoke the same guarded close handler used for the
Escape/close-button path (i.e. call the component/method that performs the
discard-confirmation/crop-exit logic) instead of mutating
compactEditor.uiStateRef.isEditorOpen directly, and keep event.accepted = true
and the Qt.Key_E check unchanged.
- Around line 227-231: The save shortcut currently always calls
compactEditor.controllerRef.save_edited_image() even while a save is in
progress; modify the Qt.Key_S branch to respect uiStateRef.isSaving by checking
uiStateRef.isSaving and only invoking
compactEditor.controllerRef.save_edited_image() when it is false (you can still
call compactEditor.ensureEditorLoaded() unconditionally or before the guard),
and ensure event.accepted is set as before; reference
compactEditor.ensureEditorLoaded(),
compactEditor.controllerRef.save_edited_image(), and uiStateRef.isSaving when
making this change.
---
Nitpick comments:
In `@faststack/imaging/metadata.py`:
- Around line 173-211: The two functions _exif_rational_to_float and
_exif_rational_to_seconds contain duplicate conversion and logging logic;
extract a single shared helper (e.g., _parse_exif_rational or
_exif_rational_to_number) that performs the numerator/denominator and tuple/list
handling and final float() conversion and logging, then have both
_exif_rational_to_float and _exif_rational_to_seconds delegate to that helper
(or have one thin wrapper if different semantics are needed), ensuring both use
the same logging message/format to eliminate duplication and keep behavior
consistent.
🪄 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: da6c195d-243d-49b3-a671-1dc487f0b06b
📒 Files selected for processing (14)
ChangeLog.mdREADME.mdfaststack/app.pyfaststack/config.pyfaststack/imaging/editor.pyfaststack/imaging/math_utils.pyfaststack/imaging/metadata.pyfaststack/qml/CompactEditorWindow.qmlfaststack/qml/Components.qmlfaststack/qml/ImageEditorDialog.qmlfaststack/qml/Main.qmlfaststack/qml/SettingsDialog.qmlfaststack/ui/provider.pypyproject.toml
Comment on lines
+2576
to
+2586
| auto_vibrance_delta = 0.0 | ||
| if include_auto_vibrance and self.auto_vibrance_enabled: | ||
| auto_vibrance_delta = self.image_editor.analyze_auto_vibrance( | ||
| blacks=float(recommendation["base_blacks"]), | ||
| whites=float(recommendation["base_whites"]), | ||
| ) | ||
|
|
||
| base_vibrance = max( | ||
| -1.0, | ||
| min(1.0, current_vibrance + auto_vibrance_delta), | ||
| ) |
There was a problem hiding this comment.
Report the effective vibrance change after clamping.
base_vibrance is clamped to [-1.0, 1.0], but auto_vibrance_delta keeps the raw analysis result. If the current vibrance is already near the limit, the preview/save applies less change than the status text reports. Example: 0.95 + 0.20 clamps to 1.00, but later messaging still says vibrance +0.20.
Suggested fix
auto_vibrance_delta = 0.0
if include_auto_vibrance and self.auto_vibrance_enabled:
auto_vibrance_delta = self.image_editor.analyze_auto_vibrance(
blacks=float(recommendation["base_blacks"]),
whites=float(recommendation["base_whites"]),
)
- base_vibrance = max(
+ base_vibrance = max(
-1.0,
min(1.0, current_vibrance + auto_vibrance_delta),
)
+ auto_vibrance_delta = base_vibrance - current_vibrance
return ActiveAutoAdjustState(📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| auto_vibrance_delta = 0.0 | |
| if include_auto_vibrance and self.auto_vibrance_enabled: | |
| auto_vibrance_delta = self.image_editor.analyze_auto_vibrance( | |
| blacks=float(recommendation["base_blacks"]), | |
| whites=float(recommendation["base_whites"]), | |
| ) | |
| base_vibrance = max( | |
| -1.0, | |
| min(1.0, current_vibrance + auto_vibrance_delta), | |
| ) | |
| auto_vibrance_delta = 0.0 | |
| if include_auto_vibrance and self.auto_vibrance_enabled: | |
| auto_vibrance_delta = self.image_editor.analyze_auto_vibrance( | |
| blacks=float(recommendation["base_blacks"]), | |
| whites=float(recommendation["base_whites"]), | |
| ) | |
| base_vibrance = max( | |
| -1.0, | |
| min(1.0, current_vibrance + auto_vibrance_delta), | |
| ) | |
| auto_vibrance_delta = base_vibrance - current_vibrance |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@faststack/app.py` around lines 2576 - 2586, The reported vibrance delta can
differ from the applied value because base_vibrance is clamped; update the logic
after computing base_vibrance so the effective change is computed and used for
messaging or saving. Specifically, after calling
self.image_editor.analyze_auto_vibrance (and after calculating base_vibrance),
compute effective_delta = base_vibrance - current_vibrance (or set
auto_vibrance_delta = base_vibrance - current_vibrance) and use that
effective_delta when generating status text or persisting changes so messages
reflect the clamped result rather than the raw analysis.
Comment on lines
+3016
to
+3018
| if self.ui_state.isCropping: | ||
| self.update_status_message("Apply or cancel the crop before saving") | ||
| return |
There was a problem hiding this comment.
Move this crop-save block into the shared save-preparation path.
This slot now blocks direct saves, but Line 2719 still auto-cancels crop transactions inside _prepare_current_session_save_request(), which is also used by Line 2982 (start_drag_current_image) and Line 2997 (prepare_for_app_close). Closing the app or starting a drag during crop mode will still silently drop the crop selection and continue saving the rest of the session, so the new invariant is only partially enforced.
Suggested fix
def _prepare_current_session_save_request(
self,
*,
editor_was_open: bool,
success_message: Optional[str],
) -> Optional[dict[str, Any]]:
"""Capture an immutable save request for the current live editor session."""
self._last_save_prepare_error = None
- if self._crop_mode_has_saved_geometry or (
- self.ui_state and getattr(self.ui_state, "isCropping", False) is True
- ):
- self._cancel_crop_transaction_for_session_boundary()
+ if self._crop_mode_has_saved_geometry or (
+ self.ui_state and getattr(self.ui_state, "isCropping", False) is True
+ ):
+ self._last_save_prepare_error = "Apply or cancel the crop before saving"
+ self.update_status_message(self._last_save_prepare_error, timeout=5000)
+ return None📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if self.ui_state.isCropping: | |
| self.update_status_message("Apply or cancel the crop before saving") | |
| return | |
| def _prepare_current_session_save_request( | |
| self, | |
| *, | |
| editor_was_open: bool, | |
| success_message: Optional[str], | |
| ) -> Optional[dict[str, Any]]: | |
| """Capture an immutable save request for the current live editor session.""" | |
| self._last_save_prepare_error = None | |
| if self._crop_mode_has_saved_geometry or ( | |
| self.ui_state and getattr(self.ui_state, "isCropping", False) is True | |
| ): | |
| self._last_save_prepare_error = "Apply or cancel the crop before saving" | |
| self.update_status_message(self._last_save_prepare_error, timeout=5000) | |
| return None |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@faststack/app.py` around lines 3016 - 3018, The crop-save guard (the if
self.ui_state.isCropping ... update_status_message ... return) must be moved out
of the ad-hoc save path and into the centralized save-preparation function
_prepare_current_session_save_request() so saving is consistently blocked while
a crop is active; update _prepare_current_session_save_request() to check
self.ui_state.isCropping, call update_status_message("Apply or cancel the crop
before saving") and return a failure/abort result instead of silently
auto-cancelling, and remove any duplicate auto-cancel logic so callers like
start_drag_current_image and prepare_for_app_close rely on the centralized
behavior.
…_duplicate filename Make compact editor arrow key handling reliable when child controls have focus
Revise auto-levels to use a true clip budget, luminance-driven endpoints, midtone correction, soft highlight/shadow rolloff, and optional export dithering for strong stretches. Improve auto white balance by analyzing the cropped area, blending neutral-pixel and Shades-of-Gray estimates, scaling correction by confidence, damping tint shifts, and normalizing WB gains to preserve luminance. Add settings for the new auto-levels/AWB behavior, expose brightness in the compact editor, and document the improved quick auto-adjust behavior. Make RAW development safer by preventing duplicate in-flight develops, writing RawTherapee output to a temporary TIFF first, and atomically replacing the working TIFF only after successful validation.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
ChangeLog.md (2)
15-15: 💤 Low valueStyle: Reduce word repetition in consecutive sentences.
Three successive sentences beginning with "Auto white balance" (lines 14–16) reduce readability. Consider rephrasing one or two to vary the sentence openers:
- Auto white balance scales its correction by confidence: few usable neutral pixels, or disagreement between its two estimators, fades the correction toward neutral instead of over-correcting scenes that lack true grays. - - Auto white balance blends a second, independent estimator (Shades-of-Gray, Minkowski p=6) with the neutral-pixel estimate; agreement between the two raises confidence and blending tempers each one's biases. + - It also blends a second, independent estimator (Shades-of-Gray, Minkowski p=6) with the neutral-pixel estimate; agreement between the two raises confidence and blending tempers each one's biases.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ChangeLog.md` at line 15, In the ChangeLog.md file around lines 14-16, there are three consecutive sentences that all begin with "Auto white balance", creating repetitive sentence structure that reduces readability. Rephrase one or two of these sentences to vary the opening words while preserving the technical meaning and content. This will improve the flow and readability of the changelog entry.Source: Linters/SAST tools
39-39: 💤 Low valueStyle: Use hyphenated compound adjective before noun.
Line 39 uses
~1 secondbefore the nounstall. Per style guides, the tilde should be dropped and the compound should be hyphenated:1-second UI-thread stall.- This removes a ~1 second UI-thread stall after every save. + This removes a 1-second UI-thread stall after every save.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ChangeLog.md` at line 39, In the ChangeLog.md entry describing batch image additions, change the text from `~1 second UI-thread stall` to `1-second UI-thread stall`. Remove the tilde (~) before the number and hyphenate the compound adjective (1-second) that precedes the noun "stall" to follow proper style guide conventions for compound adjectives.Source: Linters/SAST tools
faststack/imaging/prefetch.py (1)
453-466: 💤 Low valueConsider adding
strict=Trueto thezip()call for defensive coding.While the length equality check on line 453 makes this safe, adding
strict=Truewould provide an extra guard against future refactoring that might break this invariant.changed = [ - i for i, (a, b) in enumerate(zip(old, image_files)) if a != b + i for i, (a, b) in enumerate(zip(old, image_files, strict=True)) if a != b ]🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@faststack/imaging/prefetch.py` around lines 453 - 466, In the list comprehension on line 454 where zip(old, image_files) is called inside enumerate, add the strict=True parameter to the zip() function. This will ensure that if old and image_files ever have different lengths in the future, a ValueError will be raised instead of silently truncating to the shorter sequence, providing defensive protection against refactoring that might break the length equality invariant checked on line 453.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ChangeLog.md`:
- Line 45: The release date for version 1.6.4 is inconsistent between
ChangeLog.md and README.md. The ChangeLog shows (2026-06-06) while README shows
June 7, 2026, and the PR creation timestamp indicates 2026-06-07. Align the date
across both files by updating the date in the version 1.6.4 header to match the
correct release date (2026-06-07 based on the PR timestamp), and ensure the
corresponding date in README.md is also consistent with this aligned date.
In `@faststack/imaging/editor.py`:
- Around line 93-103: The _rec601_gray function has inconsistent output shapes
between two code paths: the cv2.transform call returns shape (H, W, 1) while the
fallback arr @ _REC601_LUMA returns shape (H, W). Since callers append [...,
None] to the result expecting 2D input, the cv2 path will produce 4D tensors and
break broadcasting. Modify the cv2.transform return statement to squeeze or
reshape the output to 2D (removing the singleton third dimension) so both code
paths return consistent (H, W) shaped arrays.
In `@faststack/ui/provider.py`:
- Around line 1233-1242: The originalCompareActive setter updates only
UIState._original_compare_active, but ImageProvider.requestImage() checks
app_controller._original_compare_active for the actual rendering decision,
causing a state divergence. In the originalCompareActive setter, after updating
self._original_compare_active and emitting the signal, also update
self.app_controller._original_compare_active to the same new_value to ensure the
controller and UI state remain synchronized.
---
Nitpick comments:
In `@ChangeLog.md`:
- Line 15: In the ChangeLog.md file around lines 14-16, there are three
consecutive sentences that all begin with "Auto white balance", creating
repetitive sentence structure that reduces readability. Rephrase one or two of
these sentences to vary the opening words while preserving the technical meaning
and content. This will improve the flow and readability of the changelog entry.
- Line 39: In the ChangeLog.md entry describing batch image additions, change
the text from `~1 second UI-thread stall` to `1-second UI-thread stall`. Remove
the tilde (~) before the number and hyphenate the compound adjective (1-second)
that precedes the noun "stall" to follow proper style guide conventions for
compound adjectives.
In `@faststack/imaging/prefetch.py`:
- Around line 453-466: In the list comprehension on line 454 where zip(old,
image_files) is called inside enumerate, add the strict=True parameter to the
zip() function. This will ensure that if old and image_files ever have different
lengths in the future, a ValueError will be raised instead of silently
truncating to the shorter sequence, providing defensive protection against
refactoring that might break the length equality invariant checked on line 453.
🪄 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: 07d558c8-4533-4212-9b3b-1fd14c3892d5
📒 Files selected for processing (22)
ChangeLog.mdREADME.mdfaststack/app.pyfaststack/config.pyfaststack/imaging/editor.pyfaststack/imaging/math_utils.pyfaststack/imaging/orientation.pyfaststack/imaging/prefetch.pyfaststack/io/sidecar.pyfaststack/io/watcher.pyfaststack/models.pyfaststack/qml/CompactEditorWindow.qmlfaststack/qml/Components.qmlfaststack/qml/ImageEditorDialog.qmlfaststack/qml/Main.qmlfaststack/qml/MenuActionItem.qmlfaststack/qml/SettingsDialog.qmlfaststack/tests/dummy_images/faststack.jsonfaststack/tests/test_prefetch_concurrency.pyfaststack/thumbnail_view/model.pyfaststack/ui/keystrokes.pyfaststack/ui/provider.py
✅ Files skipped from review due to trivial changes (2)
- faststack/tests/dummy_images/faststack.json
- faststack/config.py
| - Navigation metadata lookups (filename/uploaded/stack/batch status shown per image) no longer run the sidecar legacy-key migration scan, which was doing hundreds of filesystem checks per navigated image on the GUI thread during fast arrow-key scrolling. | ||
| - Saving an image no longer invalidates the entire decode cache. Saves, watcher-detected file changes, and edit-revert now invalidate only the files that actually changed, so navigation right after a save no longer hits a blocking re-decode and the prefetch window stays warm (previously every save re-decoded ~9 neighboring images and bumped the global cache generation several times). A per-path invalidation epoch makes this safe: decode results that started reading a file before it was replaced are discarded instead of re-inserting stale pixels. The global invalidation still applies for resize/zoom/color-mode changes and unattributable directory events. | ||
|
|
||
| ## 1.6.4 (2026-06-06) |
There was a problem hiding this comment.
Unresolved date inconsistency: ChangeLog vs. README.
Line 45 shows (2026-06-06), but README line 3 shows June 7, 2026, and the PR was created on 2026-06-07T17:20:40Z. A previous review flagged this inconsistency. Please align the release date across both files: either update the ChangeLog to 2026-06-07 or verify and document why the ChangeLog date should differ.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@ChangeLog.md` at line 45, The release date for version 1.6.4 is inconsistent
between ChangeLog.md and README.md. The ChangeLog shows (2026-06-06) while
README shows June 7, 2026, and the PR creation timestamp indicates 2026-06-07.
Align the date across both files by updating the date in the version 1.6.4
header to match the correct release date (2026-06-07 based on the PR timestamp),
and ensure the corresponding date in README.md is also consistent with this
aligned date.
Comment on lines
+1233
to
+1242
| @Property(bool, notify=originalCompareActiveChanged) | ||
| def originalCompareActive(self) -> bool: | ||
| return self._original_compare_active | ||
|
|
||
| @originalCompareActive.setter | ||
| def originalCompareActive(self, new_value: bool): | ||
| if self._original_compare_active != new_value: | ||
| self._original_compare_active = new_value | ||
| self.originalCompareActiveChanged.emit(new_value) | ||
|
|
There was a problem hiding this comment.
Unify originalCompareActive state with the controller source of truth.
This setter only updates UIState._original_compare_active, but ImageProvider.requestImage() gates compare-preview rendering on self.app_controller._original_compare_active. That split can make UI state and rendered source diverge.
💡 Proposed fix
`@Property`(bool, notify=originalCompareActiveChanged)
def originalCompareActive(self) -> bool:
- return self._original_compare_active
+ return bool(getattr(self.app_controller, "_original_compare_active", False))
`@originalCompareActive.setter`
def originalCompareActive(self, new_value: bool):
- if self._original_compare_active != new_value:
- self._original_compare_active = new_value
- self.originalCompareActiveChanged.emit(new_value)
+ new_value = bool(new_value)
+ current = bool(getattr(self.app_controller, "_original_compare_active", False))
+ if current != new_value:
+ setattr(self.app_controller, "_original_compare_active", new_value)
+ self._original_compare_active = new_value
+ self.originalCompareActiveChanged.emit(new_value)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@faststack/ui/provider.py` around lines 1233 - 1242, The originalCompareActive
setter updates only UIState._original_compare_active, but
ImageProvider.requestImage() checks app_controller._original_compare_active for
the actual rendering decision, causing a state divergence. In the
originalCompareActive setter, after updating self._original_compare_active and
emitting the signal, also update self.app_controller._original_compare_active to
the same new_value to ensure the controller and UI state remain synchronized.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/build-executables.yml:
- Around line 32-35: Replace the mutable version tag references in the GitHub
Actions workflow with immutable commit SHAs. For the actions/checkout action and
the actions/setup-python action, replace the `@v6` tag with the corresponding full
commit SHA to ensure supply-chain integrity and prevent unexpected behavior from
version updates. This ensures the workflow uses the exact version of each action
that was tested and approved.
- Around line 31-33: The checkout step in the build-executables workflow
currently does not disable persisted credentials, which leaves a token in the
git config for later steps in the workflow. Add the `persist-credentials: false`
parameter to the `uses: actions/checkout@v6` step to prevent the checkout action
from storing credentials that could be misused by subsequent steps.
🪄 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: 3408cd5e-899e-4197-935e-6f0bad06a5c3
📒 Files selected for processing (9)
.github/workflows/build-executables.yml.gitignorefaststack/app.pyfaststack/imaging/editor.pyfaststack/imaging/turbo.pyfaststack/resources.pyfaststack/ui/provider.pypackaging/faststack.specpyproject.toml
🚧 Files skipped from review as they are similar to previous changes (1)
- faststack/ui/provider.py
Owner
Author
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 941ab45306
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| binaries.append(binary) | ||
| break | ||
|
|
||
| hiddenimports = [ |
There was a problem hiding this comment.
When any TurboJPEG candidate is found (for example via FASTSTACK_TURBOJPEG_LIB or a standard Homebrew path), the break above exits the loop before this in-loop assignment runs, so Analysis(..., hiddenimports=hiddenimports, ...) later raises because hiddenimports was never defined. Keep hiddenimports outside the candidate loop so executable builds work on machines that actually have the library installed.
Useful? React with 👍 / 👎.
Comment on lines
+2558
to
+2563
| "base_whites": float(state.base_whites), | ||
| "p_low": float(state.p_low), | ||
| "p_high": float(state.p_high), | ||
| "base_vibrance": float(state.base_vibrance), | ||
| "auto_vibrance_delta": float(state.auto_vibrance_delta), | ||
| "extra_highlight_steps": int(state.extra_highlight_steps), |
There was a problem hiding this comment.
The saved auto-adjust state now includes base_brightness in memory, and restored sessions reuse that state for the reduce/raise_auto_adjust_* controls, but this serializer only writes the vibrance and level fields. After saving/navigating/reopening an auto-leveled image with a midtone brightness correction, pressing one of those adjustment controls restores a valid ActiveAutoAdjustState with base_brightness defaulting to 0.0, so _apply_auto_adjust_preview() resets the brightness slider and drops the midtone correction.
Useful? React with 👍 / 👎.
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
2 participants
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.
save_edited_image() and set_edit_parameter() now block while crop mode is active, which matches the new README guidance that crop must be applied or cancelled before saving/editing.
Crop mode is now allowed while the compact editor is open, but still blocked when the expanded editor is open. That is an intentional behavior change and the guard looks localized.
EXIF brief cache keys now include both current and previous image source keys so the “distance from previous photo” is order-sensitive. That is the right direction for filtered/sorted views, because the displayed previous image matters.
Rotation now transforms an existing normalized crop box when 90-degree rotation changes. The transform logic looks plausible for normalized 0–1000 coordinates and should fix the “rotate already-cropped image” class of bugs.
np.ascontiguousarray(arr_u8) before QImage serialization is a solid correctness fix for rotated/non-contiguous NumPy views.
Highlight recovery and exposure/whites slider scaling are real visual-output changes, not cleanup. They appear intentional based on the changelog and README.
Summary by CodeRabbit
Release Notes - v1.6.4
New Features
Bug Fixes
Performance & Stability
Documentation