Skip to content

fix: address repo review findings - #2

Merged
danjdewhurst merged 2 commits into
mainfrom
claude/repo-review-b5721d
Jun 11, 2026
Merged

danjdewhurst merged 2 commits into
mainfrom
claude/repo-review-b5721d

Conversation

@danjdewhurst

Copy link
Copy Markdown
Contributor

Fixes the findings from a full repo review. No new features — performance, correctness, and honesty cleanups, plus CI.

Performance & correctness

  • Render previews off the main actor: renderPreviewIfNeeded now runs the expensive CIContext rasterization and histogram build in a detached task; only debounce, generation checks, and state publishing stay on the MainActor. Previously the whole render ran on the main thread and blocked the UI during edits.
  • Grain size was inverted: noise is now scaled by size instead of 1/size, so larger grain sizes produce coarser grain.
  • Color pipeline precision: sources load as half-float (.RGBAh) instead of 8-bit, and the default extended_linear_srgb working space now maps to the extended linear sRGB color space instead of gamma-encoded extended sRGB.
  • Histogram clipping counts a pixel only when all channels clip, so saturated colors no longer over-report shadow/highlight clipping.
  • Import no longer double-decodes: photos are validated with a cheap ImageIO check on import/relink; the full decode happens once when the item is applied.
  • openProject sets the project's selected item, so edits to the auto-selected photo persist without manually reselecting it.
  • Export name preview is formatted without probing the filesystem (it previously ran fileExists checks against /tmp from the view body and could show a misleading -2 suffix).
  • Preview cache is bounded by ~256 MB of estimated pixel bytes in addition to the existing 8-entry limit.
  • Stale-index crash risk in the Local inspector section: layer reads go through a guarded accessor and binding setters validate indices.

Cleanup

  • Removed the inaccessible .sideBySide comparison mode (it was hidden from the UI via a custom allCases override and rendered the split view anyway); README updated to match.
  • Removed the *CoverageProbe enums from production view code and the coverage ignore-regex from script/test.sh, so reported coverage no longer excludes the largest source files.
  • Consolidated the duplicated batch-export state construction into shared helpers used by both the async and synchronous drivers.
  • Labeled the derived calibration readouts as preview placeholders in the inspector.

CI

  • Added .github/workflows/ci.yml running swift build and ./script/test.sh on macos-15.

Notes for review

This branch was authored in a Linux environment without a Swift toolchain, so it has not been compiled locally — the new CI workflow on this PR is the first build. Two changes intentionally alter observable behavior that tests don't pin and deserve a quick visual check: grain size direction and the clipping ratios.

https://claude.ai/code/session_018WfrSbcryCLDHBqAQHawki


Generated by Claude Code

claude added 2 commits June 10, 2026 22:15
- Render previews on a detached task instead of the main actor, so
  CIContext rasterization no longer blocks the UI during edits
- Bound the preview render cache by estimated bytes (256 MB) in
  addition to the entry count
- Scale grain noise by grain size instead of its inverse, so larger
  sizes produce coarser grain
- Load sources as half-float and map extended_linear_srgb to the
  extended linear sRGB color space instead of gamma-encoded extended
  sRGB, preserving precision for >8-bit sources
- Count histogram clipping only when all channels clip, so saturated
  colors no longer over-report shadow/highlight clipping
- Validate photos on import via ImageIO instead of fully decoding
  them twice
- Set the project's selected item when opening a project so edits
  persist without reselecting
- Format the export name preview without probing the filesystem
- Remove the inaccessible side-by-side comparison mode
- Guard local adjustment layer bindings against stale indices
- Label derived calibration readouts as placeholders in the inspector
- Share batch export state construction between the async and
  synchronous export drivers
- Remove view coverage probes and report coverage without ignoring
  the largest source files

https://claude.ai/code/session_018WfrSbcryCLDHBqAQHawki
@coderabbitai

coderabbitai Bot commented Jun 11, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@danjdewhurst, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 58 minutes and 16 seconds. Learn how PR review limits work.

Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 57058347-bef2-464f-8972-613205bec500

📥 Commits

Reviewing files that changed from the base of the PR and between 8d662c2 and 6ff5791.

📒 Files selected for processing (14)
  • .github/workflows/ci.yml
  • README.md
  • Sources/FilmChefCore/Models/EditorModels.swift
  • Sources/FilmChefCore/Services/FilmPipelineRenderer.swift
  • Sources/FilmChefCore/Services/ImageProcessor.swift
  • Sources/FilmChefCore/Stores/EditorStore+ExportNaming.swift
  • Sources/FilmChefCore/Stores/EditorStore.swift
  • Sources/FilmChefCore/Views/ContentView.swift
  • Sources/FilmChefCore/Views/ControlsView.swift
  • Sources/FilmChefCore/Views/HistogramView.swift
  • Sources/FilmChefCore/Views/PreviewPaneView.swift
  • Sources/FilmChefCore/Views/SidebarView.swift
  • Tests/FilmChefCoreTests/main.swift
  • script/test.sh

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.

@danjdewhurst
danjdewhurst merged commit 494a388 into main Jun 11, 2026
1 of 2 checks passed
@danjdewhurst
danjdewhurst deleted the claude/repo-review-b5721d branch June 11, 2026 06:31
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.

2 participants