Skip to content

feat(insights): Add timeout to image optimization analysis #614

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
NicoHinderling merged 2 commits into from
May 8, 2026
+20 −1
Merged

feat(insights): Add timeout to image optimization analysis #614

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2c51937
feat(insights): Add timeout to image optimization analysis
NicoHinderling May 8, 2026
0259e3a
fix(insights): Prevent false timed_out flag when all images complete
NicoHinderling May 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 19 additions & 1 deletion src/launchpad/size/insights/apple/image_optimization.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import io
import logging
import time

from abc import ABC, abstractmethod
from concurrent.futures import ThreadPoolExecutor, as_completed
Expand Down Expand Up @@ -46,6 +47,7 @@ class BaseImageOptimizationInsight(Insight[ImageOptimizationInsightResult], ABC)
TARGET_JPEG_QUALITY = 85
TARGET_HEIC_QUALITY = 85
_MAX_WORKERS = 4
_TIMEOUT_SECONDS = 300

@abstractmethod
def _find_images(self, input: InsightsInput) -> List[FileInfo]:
Expand Down Expand Up @@ -73,17 +75,32 @@ def generate(self, input: InsightsInput) -> ImageOptimizationInsightResult | Non
return None

results: List[OptimizableImageFile] = []
timed_out = False
completed = 0
Copy link
Copy Markdown
Member

@trevor-e trevor-e May 8, 2026

Choose a reason for hiding this comment

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

nit: is completed helpful here vs len(results)?

Copy link
Copy Markdown
Contributor Author

@NicoHinderling NicoHinderling May 8, 2026

Choose a reason for hiding this comment

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

iirc The completed counter tracks all processed futures (including failures and below-threshold images), while len(results) only counts optimization candidates

deadline = time.monotonic() + self._TIMEOUT_SECONDS
with ThreadPoolExecutor(max_workers=min(self._MAX_WORKERS, len(files))) as executor:
future_to_file = {executor.submit(self._analyze_image_optimization, f): f for f in files}
for future in as_completed(future_to_file):
completed += 1
Copy link
Copy Markdown

@cursor cursor Bot May 8, 2026

Choose a reason for hiding this comment

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

Deadline not enforced while blocked in as_completed

Low Severity

The as_completed(future_to_file) call has no timeout parameter, so __next__() blocks indefinitely waiting for the next future. The deadline check on line 91 only runs after a future completes. If all _MAX_WORKERS running futures are slow (e.g. processing very large images), the code remains stuck inside as_completed well past the 300-second deadline. Passing a timeout to as_completed and catching TimeoutError would enforce the deadline even while waiting.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 0259e3a. Configure here.

try:
result = future.result()
if result and result.potential_savings >= self.MIN_SAVINGS_THRESHOLD:
results.append(result)
except Exception: # pragma: no cover
logger.exception("Failed to analyze image in thread pool")
if completed < len(files) and time.monotonic() >= deadline:
timed_out = True
logger.warning(
"size.insight.image_optimization.timeout | completed=%d/%d timeout=%ds",
completed,
len(files),
self._TIMEOUT_SECONDS,
)
for fut in future_to_file:
fut.cancel()
break
Comment thread
cursor[bot] marked this conversation as resolved.

if not results:
if not results and not timed_out:
return None

results.sort(key=lambda x: x.potential_savings, reverse=True)
Expand All @@ -92,6 +109,7 @@ def generate(self, input: InsightsInput) -> ImageOptimizationInsightResult | Non
return ImageOptimizationInsightResult(
optimizable_files=results,
total_savings=total_savings,
timed_out=timed_out,
)

def _analyze_image_optimization(
Expand Down
1 change: 1 addition & 0 deletions src/launchpad/size/models/insights.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,7 @@ class ImageOptimizationInsightResult(BaseInsightResult):
optimizable_files: List[OptimizableImageFile] = Field(
..., description="Files that can be optimized with potential savings"
)
timed_out: bool = Field(False, description="Whether analysis was cut short due to timeout")

def get_file_paths(self) -> List[str]:
Copy link
Copy Markdown
Contributor

@sentry sentry Bot May 8, 2026

Choose a reason for hiding this comment

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

Bug: The frontend does not handle the new timed_out field for image optimization insights, leading to a misleading UI when an analysis times out.
Severity: MEDIUM

Suggested Fix

Update the ImageOptimizationInsightResult interface in web/src/utils/dataConverter.ts to include the timed_out field. Modify the InsightsDisplay.tsx component to check this field and render a warning message indicating that the analysis timed out and results are incomplete.

Prompt for AI Agent 
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: src/launchpad/size/models/insights.py#L218

Potential issue: The backend `ImageOptimizationInsightResult` model was updated to
include a `timed_out` boolean field to signal when an analysis is cut short. However,
the corresponding frontend TypeScript interface and React component were not updated to
consume this new field. Consequently, if an analysis times out, especially before
processing any files, the frontend receives `total_savings=0` and an empty
`optimizable_files` array. It then incorrectly displays a "No savings" message,
misleading the user into thinking the analysis completed successfully when it actually
timed out.

Also affects:

  • web/src/utils/dataConverter.ts
  • web/src/components/InsightsDisplay.tsx

Did we get this right? 👍 / 👎 to inform future reviews.

return [f.file_path for f in self.optimizable_files]
Expand Down
Loading