Skip to content

Update histogram#29

Merged
AlanRockefeller merged 2 commits intomainfrom
test
Jan 3, 2026
Merged

Update histogram#29
AlanRockefeller merged 2 commits intomainfrom
test

Conversation

@AlanRockefeller
Copy link
Copy Markdown
Owner

@AlanRockefeller AlanRockefeller commented Jan 3, 2026

Summary by CodeRabbit

Release Notes

  • Bug Fixes

    • Histogram computation now operates asynchronously without blocking the main thread, significantly improving application responsiveness during image analysis.
    • Enhanced image decoding for background operations with improved error handling and graceful fallback mechanisms.
  • Chores

    • Added test infrastructure with dummy image configuration files.
    • Updated repository configuration and ignore patterns.

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

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jan 3, 2026

Walkthrough

This PR introduces a thread-safe image decoding helper to support background histogram computation without blocking the main thread. It refactors the histogram pipeline to safely fetch decoded images on-demand via the prefetcher, updates the histogram worker signature to support fallback decoding, and adds test data. Documentation describing ICC color profile handling is removed and ignored.

Changes

Cohort / File(s) Summary
Configuration & Documentation
.gitignore
Added docs/COLOR_PROFILE_FIX.md to ignored paths
Removed Documentation
docs/COLOR_PROFILE_FIX.md
Deleted documentation file describing ICC color profile fix for JPEG decoding, including color management implementation details and testing guidelines
Histogram Pipeline & Safe Decoding
faststack/app.py
Introduced _get_decoded_image_safe(index) for thread-safe background image fetching; refactored histogram handling with target_index logic to avoid main thread blocking; extended _compute_histogram_worker signature to accept controller and target_index for on-demand fallback decoding; added error handling for safe retry/skip behavior
Test Data
faststack/tests/dummy_images/faststack.json
Added new JSON test fixture with versioned stack data structure (version 2) containing single test entry and empty stacks array

Sequence Diagram(s)

sequenceDiagram
    participant MT as Main Thread
    participant HC as Histogram<br/>Controller
    participant HW as Histogram<br/>Worker
    participant PF as Prefetcher
    participant Cache as Image Cache

    MT->>HC: Trigger histogram update
    
    rect rgb(200, 220, 255)
    Note over HC: Check if preview data available
    alt Preview data available
        HC->>HW: _compute_histogram_worker<br/>(decoded provided)
    else Preview data unavailable
        HC->>HC: Compute target_index
        HC->>HW: _compute_histogram_worker<br/>(controller, target_index)
    end
    end
    
    HW->>Cache: Check cache hit
    alt Cache miss
        HW->>HC: _get_decoded_image_safe(target_index)
        HC->>PF: Delegate decode with priority
        PF-->>HC: Return decoded (or None)
        HC-->>HW: DecodedImage
    else Cache hit
        Cache-->>HW: DecodedImage
    end
    
    rect rgb(220, 240, 220)
    Note over HW: Compute histogram
    HW->>HW: Safe fallback if data unavailable
    end
    
    HW-->>MT: Histogram result
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • Fix histogram #28: Modifies histogram pipeline with safer fallback/preview-fetch behavior and changes how histogram worker obtains decoded image data.
  • Test #27: Changes histogram/preview submission and retry logic in background worker handling.
  • Update image editor #26: Introduces/kicks histogram workers and modifies _compute_histogram_worker signature in AppController's background pipeline.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Title check ❓ Inconclusive The title 'Update histogram' is vague and generic. While histogram-related changes exist in faststack/app.py, the PR also includes unrelated changes (.gitignore, removed documentation file, and test JSON file), making the title incomplete and potentially misleading about the full scope of changes. Revise the title to either more specifically describe the histogram changes (e.g., 'Refactor histogram worker to support safe image decoding') or provide a more comprehensive title that captures the broader scope of all modifications in the changeset.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • 📝 Generate docstrings

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.

❤️ Share

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

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 52a6abc and 1514093.

📒 Files selected for processing (4)
  • .gitignore
  • docs/COLOR_PROFILE_FIX.md
  • faststack/app.py
  • faststack/tests/dummy_images/faststack.json
💤 Files with no reviewable changes (1)
  • docs/COLOR_PROFILE_FIX.md
🧰 Additional context used
🧬 Code graph analysis (1)
faststack/app.py (3)
faststack/models.py (1)
  • DecodedImage (37-46)
faststack/imaging/cache.py (1)
  • build_cache_key (81-87)
faststack/imaging/prefetch.py (1)
  • submit_task (261-301)
🪛 Ruff (0.14.10)
faststack/app.py

596-596: Do not catch blind exception: Exception

(BLE001)

🔇 Additional comments (4)
.gitignore (1)

31-31: LGTM! Documentation exclusion is appropriate.

The addition aligns with the removal of docs/COLOR_PROFILE_FIX.md and follows the existing pattern for documentation files.

faststack/tests/dummy_images/faststack.json (1)

1-18: LGTM! Test fixture is well-structured.

The JSON structure provides a clean baseline for testing with a minimal entry and empty stacks, which is appropriate for test data.

faststack/app.py (2)

701-720: Design choice: Histogram worker can now block on image decode.

The updated _kick_histogram_worker passes the controller and target_index to enable the histogram worker to fetch and decode images when preview data isn't available (Lines 704-706, 720). This means the histogram worker thread may block waiting for image decode (via _get_decoded_image_safeprefetcher.submit_taskfuture.result(timeout=5.0)).

Trade-off: This provides a robust fallback for histogram computation when the editor preview isn't ready, but could delay histogram updates by up to 5 seconds on slow decodes. The histogram throttling (50ms timer, line 207) helps mitigate UX impact.

This is a reasonable design choice for this use case where histogram accuracy is prioritized over instant updates.


727-734: Fallback decode path provides robustness for histogram computation.

The updated worker signature allows fetching decoded data on-demand via controller._get_decoded_image_safe(target_index) when preview data isn't available (Lines 732-733). This ensures histograms can be computed even when the editor preview hasn't been rendered yet.

The implementation correctly checks for both missing decoded data and a valid target_index before attempting the fetch (Line 732).

Comment thread faststack/app.py
@AlanRockefeller AlanRockefeller merged commit bde9a6b into main Jan 3, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant