feat(tools): RAW tuner#284
Open
webbertakken wants to merge 11 commits into
Open
Conversation
jsdom@29 / whatwg-url@16 / html-encoding-sniffer@5+ all migrated to @exodus/bytes which is ESM-only. Vitest 4.1.5's CJS config loader cannot require() it, so the test suite fails to start. Pin jsdom to ^26 (uses whatwg-encoding) and rename vitest.config.ts to .mts so it loads via ESM regardless.
Phase 0: tools-index row, page re-export shim, component shell with smoke test, post-build chunk-isolation test, ToolPage mock for vitest.
Pure-TS pipeline (no browser APIs): SliderStack with curve and per-channel HSL, LinearImage, histogram + percentile + clipFractions + meanLuma, image analysis, auto-tune (exposure / whites / blacks / temp / tint), CPU applier with full slider chain in linear light + sRGB encode. 89 tests, 100% line coverage on every new file.
libraw-wasm wrapper (lazy-imported, with module type declaration), JPEG decode via ImageDecoder + canvas fallback (browser code carved off behind injectable BytesDecoder so jsdom unit tests cover the conversion logic), format dispatcher with extension + magic-byte sniff. 37 new tests, 100% line coverage.
WGSL compute shader mirroring the CPU slider chain byte-for-byte, pipeline wrapper with adapter probe + buffer allocation + dispatch + readback, runtime self-test on a 16-pixel fixture (tolerance 3/255), public apply() selector that picks GPU or CPU per session, prewarmGpu() to compile the pipeline before first edit. FakeDevice test harness records every call and lets tests inject synthetic readback dynamically. 45 new tests, 100% line coverage on every Phase 3 file.
ObjectStore (memory + OPFS) for model weight caching with chunked fetch + progress, transformers.js wrapper that embeds LinearImage to a 512-d feature vector via CLIP ViT-B/32 (factory + RawImage ctor injectable for tests), linear->sRGB-RGB bridge for transformers.js compatibility, progress aggregator for the multi-file model download. 35 new tests, 100% line coverage.
30 hand-curated photography presets across the mood spectrum (editorial, film noir, golden hour, vintage, cyberpunk, etc.). Build-time text-side CLIP embedder via CLIPTextModelWithProjection, L2-normalised so cosine = dot product. presets.json generated locally and committed (~15 KB). Cosine similarity + MMR diversity retrieval. tsx added to devDeps for the build script. 24 new tests, 100% line coverage.
encodeJpeg via OffscreenCanvas with injectable factory for jsdom tests, writeXmp produces Lightroom-compatible XMP sidecar (crs: namespace, full slider chain, ToneCurvePV2012 only when non-identity). @xmldom/xmldom added as a dev dep for XML round-trip assertions. 12 new tests, 100% line coverage.
DropZone (drag/drop + click + Enter/Space activation), HistogramView (RGB), SliderStack (10 scalars + reset), PresetGrid (with active state), ExportPanel (quality + JPEG/.xmp buttons), ToolBody composition root with full file -> decode -> analyse -> auto-tune -> apply -> export pipeline. transformers.js loads from CDN at runtime via webpackIgnore so Docusaurus doesn't have to bundle its Node-only deps. 47 new tests, 100% line coverage on every Phase 7 file.
Box-filter downsample so the editing preview is 1024px max while the export uses full resolution; auto-tune analysis runs on the preview because box-filter preserves histogram statistics. Applier-decision indicator at the bottom of the panel ('Rendering on the GPU / CPU'). Plan ticked for the deferred items (memory ceiling, debounce, accessibility audit, blog post).
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
WalkthroughAdds a complete RAW/JPEG tuner: domain types, decoding, CPU/WebGPU applier with self-test, heuristics and auto-tune, CLIP loading and preset retrieval, OPFS caching, JPEG/XMP export, UI components/pages, planning doc, tests, and config updates. ChangesRAW tuner end-to-end feature
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #284 +/- ##
===========================================
+ Coverage 82.00% 93.59% +11.59%
===========================================
Files 18 51 +33
Lines 500 1451 +951
Branches 96 242 +146
===========================================
+ Hits 410 1358 +948
- Misses 84 85 +1
- Partials 6 8 +2
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 16
🧹 Nitpick comments (12)
src/components/tools/RawTuner/domain/downsample.spec.ts (1)
16-21: ⚡ Quick winConsider shrinking heavy image fixtures to keep test runtime predictable.
The current dimensions are larger than needed for behaviour checks and may challenge the
<200mstest budget.Proposed patch
- const image = createLinearImage(2048, 2048) - for (let i = 0; i < 2048 * 2048; i++) { + const image = createLinearImage(512, 512) + for (let i = 0; i < 512 * 512; i++) { image.data.set([0.5, 0.5, 0.5, 1], i * PIXEL_STRIDE) } - const result = downsample(image, 1024) + const result = downsample(image, 256) @@ - const image = createLinearImage(2048, 1024) + const image = createLinearImage(512, 256) @@ - const image = createLinearImage(1024, 1) + const image = createLinearImage(256, 1)As per coding guidelines,
**/*.{test,spec}.{ts,tsx,js,jsx}: Tests must run < 200ms.Also applies to: 48-63
🤖 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 `@src/components/tools/RawTuner/domain/downsample.spec.ts` around lines 16 - 21, The test uses a very large fixture (createLinearImage(2048, 2048)) which makes downsample(image, 1024) slow; shrink the image to a much smaller size that still exercises behavior (e.g., 256x256 or 512x512) and adjust pixel fills that use PIXEL_STRIDE accordingly so the test stays under ~200ms; apply the same reduction to the other large fixture blocks referenced (around the other downsample usages at lines 48-63) and keep calls to createLinearImage, downsample, and any loops that set image.data consistent with the new dimensions.src/components/tools/RawTuner/export/encode-jpeg.ts (1)
47-56: 💤 Low valueConsider removing redundant dimension assignment.
Lines 50-51 set
canvas.widthandcanvas.heightafter the factory creates the canvas. If the factory (line 49) already sets these dimensions, the assignments are redundant. However, this defensive approach ensures correctness if a custom factory doesn't set dimensions.This is harmless but creates minor redundancy. Consider documenting the expectation or removing if the factory contract guarantees dimension setting.
♻️ Optional: Document or simplify dimension handling
Option 1: Document the expectation:
+ // Ensure dimensions are set (defensive; factory may have already set these) canvas.width = width canvas.height = heightOption 2: Remove if factory contract guarantees dimension setting:
const canvas = factory(width, height) - canvas.width = width - canvas.height = height const ctx = canvas.getContext('2d')🤖 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 `@src/components/tools/RawTuner/export/encode-jpeg.ts` around lines 47 - 56, The code defensively sets canvas.width and canvas.height after creating the canvas via factory/defaultCanvasFactory in encodeJpeg, which is redundant if the factory always initializes size; either remove the explicit assignments (canvas.width, canvas.height) and rely on the factory, or document the factory contract by adding a comment above defaultCanvasFactory/options.canvasFactory that implementations must set width/height; update encodeJpeg to either delete those two lines or add a brief comment referencing factory/defaultCanvasFactory to make the expectation explicit.src/components/tools/RawTuner/ui/DropZone.tsx (1)
12-15: ⚡ Quick winConsider adding runtime file type validation.
The
handleFilesfunction relies solely on theacceptattribute (line 77) for file type filtering, but users can bypass this by selecting "All Files" in the file picker or dropping arbitrary files. Consider adding runtime validation to ensure only supported file types are processed.🛡️ Proposed validation enhancement
const handleFiles = (files: FileList | null): void => { if (!files || files.length === 0) return + const file = files[0] + const supportedExtensions = /\.(cr2|cr3|nef|arw|dng|raf|rw2|orf|pef|jpe?g|png|webp)$/i + if (!supportedExtensions.test(file.name)) { + // Could emit an error callback or show a message + return + } - onFile(files[0]) + onFile(file) }🤖 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 `@src/components/tools/RawTuner/ui/DropZone.tsx` around lines 12 - 15, handleFiles currently trusts the input's accept attribute and passes the first file straight to onFile; add runtime validation inside handleFiles to verify the file's MIME type and/or extension against an explicit SUPPORTED_FILE_TYPES (or SUPPORTED_EXTENSIONS) list (or derive allowed types from the input's accept prop) and reject any file that doesn't match (e.g., call an onError callback, show a user message, or return early) before calling onFile; update handleFiles and ensure any helper constant (e.g., SUPPORTED_FILE_TYPES) and error handling path are implemented so unsupported files are not processed.plans/raw-tuner.md (1)
24-67: ⚡ Quick winSpecify language for fenced code block.
The fenced code block showing the directory structure should specify a language for proper syntax highlighting and to satisfy the markdownlint rule.
📝 Proposed fix
-``` +```text RawTuner/ ├── index.tsx composition root🤖 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 `@plans/raw-tuner.md` around lines 24 - 67, The fenced code block in plans/raw-tuner.md containing the directory tree (starts with "RawTuner/") lacks a language tag; update the opening fence from ``` to ```text (or another appropriate language like `bash` or `plain`) so markdownlint and syntax highlighters recognize it, i.e., edit the directory-structure code block in plans/raw-tuner.md to include the language identifier.src/components/tools/RawTuner/domain/histogram.ts (1)
102-112: ⚡ Quick winConsider documenting the asymmetric clipping logic.
The highlight clipping check (line 107) uses OR (any channel exceeding threshold), whilst the shadow clipping check (line 108) uses AND (all channels below threshold). This asymmetry is photographically correct—highlights can selectively clip in individual channels whilst shadows typically clip uniformly—but may not be immediately obvious to future maintainers.
📖 Suggested clarifying comment
for (let i = 0; i < pixels; i++) { const idx = i * PIXEL_STRIDE const r = data[idx] const g = data[idx + 1] const b = data[idx + 2] + // Highlights can selectively clip (OR), shadows typically clip uniformly (AND). if (r >= highThreshold || g >= highThreshold || b >= highThreshold) high++ if (r <= lowThreshold && g <= lowThreshold && b <= lowThreshold) low++ }🤖 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 `@src/components/tools/RawTuner/domain/histogram.ts` around lines 102 - 112, Add a brief inline comment above the pixel loop in histogram.ts explaining the asymmetric clipping logic: that the highlight check (uses r >= highThreshold || g >= highThreshold || b >= highThreshold) treats per-channel clipping because highlights can clip in individual channels, whereas the shadow check (uses r <= lowThreshold && g <= lowThreshold && b <= lowThreshold) requires all channels to be low to count as shadow clipping; reference the loop variables (data, PIXEL_STRIDE, highThreshold, lowThreshold, high, low) so future maintainers understand the intent.src/components/tools/RawTuner/decode/decode-jpeg.ts (1)
48-51: 💤 Low valueConsider using 'x' instead of '×' in error messages.
The error message uses the Unicode multiplication sign (U+00D7) which may not render correctly in all terminals or log viewers. Using lowercase 'x' would be more portable whilst remaining clear.
📝 Suggested change
if (rgba.length !== expected) { - throw new Error(`sRGB byte length ${rgba.length} does not match ${width}\u00d7${height}\u00d74`) + throw new Error(`sRGB byte length ${rgba.length} does not match ${width}x${height}x4`) }🤖 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 `@src/components/tools/RawTuner/decode/decode-jpeg.ts` around lines 48 - 51, The error message in decode-jpeg.ts builds a string using the Unicode multiplication sign (U+00D7); update the throw in the block that compares rgba.length to expected (using variables rgba, expected, width, height, PIXEL_STRIDE) to use a plain lowercase 'x' between width and height (e.g. `${width}x${height}x4`) so the message is portable across terminals and log viewers.src/components/tools/RawTuner/storage/opfs-cache.ts (1)
140-145: 💤 Low valueType cast on
response.bodyis appropriate but could use a comment.The cast to
ReadableStream<Uint8Array>at line 142 is necessary because the Fetch API types are looser than the runtime guarantee. A brief comment explaining this would help future maintainers understand why the cast is safe.📝 Suggested comment
let data: ArrayBuffer if (response.body) { + // response.body is typed broadly but is guaranteed to be ReadableStream<Uint8Array> data = await consumeStreaming( response.body as unknown as ReadableStream<Uint8Array>, total, onProgress, )🤖 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 `@src/components/tools/RawTuner/storage/opfs-cache.ts` around lines 140 - 145, Add a short inline comment above the cast where response.body is forced to ReadableStream<Uint8Array> in the block that calls consumeStreaming; explain that the Fetch API's typings are looser than the runtime in this environment and that we guarantee response.body is a ReadableStream<Uint8Array> (e.g., produced by our fetch wrapper or environment) so the cast is safe—reference the response.body cast and the call to consumeStreaming to locate the spot.src/components/tools/RawTuner/applier/webgpu/fake-device.ts (1)
31-214: ⚡ Quick winConsider adding JSDoc to test fake classes for maintainability.
Whilst the module-level JSDoc clearly explains the fake device's purpose, the individual classes (
FakeBuffer,FakeQueue,FakeComputePass,FakeCommandEncoder,FakeDevice) lack documentation. Brief class-level comments would help maintainers understand each component's role in the fake, especially for the non-obviousresolveReadbackmechanism inFakeBuffer.📝 Example JSDoc additions
+/** + * Simulates a GPU buffer with support for mapping, readback, and destruction. + * MAP_READ buffers populate `mappedReadback` via a deferred `resolveReadback` callback. + */ export class FakeBuffer {+/** + * Simulates GPU queue operations, recording writeBuffer and submit calls. + */ class FakeQueue {+/** + * Simulates a compute pass, recording pipeline, bind groups, and dispatch calls. + */ class FakeComputePass {🤖 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 `@src/components/tools/RawTuner/applier/webgpu/fake-device.ts` around lines 31 - 214, The classes in this test fake module (FakeBuffer, FakeQueue, FakeComputePass, FakeCommandEncoder, FakeDevice) lack class-level JSDoc explaining their responsibilities and important behaviors (e.g. FakeBuffer.resolveReadback), so add brief JSDoc comments above each class declaration describing its role, the meaning of key fields (mappedReadback, resolveReadback, isMapped, writes, copies, dispatched), and any lifecycle notes (e.g. mapAsync->getMappedRange->unmap, how stagingProducer/stagingBytes are applied in FakeDevice.createBuffer); reference the exact class names (FakeBuffer, FakeQueue, FakeComputePass, FakeCommandEncoder, FakeDevice) and the resolveReadback/stagingProducer interaction so reviewers can locate and verify the docs quickly.src/components/tools/RawTuner/decode/decode.ts (1)
113-113: 💤 Low valueConsider removing the no-op
liftRawMetadatafunction.The
liftRawMetadatafunction currently performs an identity transformation. If this is intentional (e.g., reserved for future metadata transformations), consider adding a comment explaining its purpose. Otherwise, you could inline the metadata directly at line 119.♻️ Optional simplification
-const liftRawMetadata = (metadata: DecodedRawMetadata): DecodedImageMetadata => metadata - export const decode = async (input: DecodeInput): Promise<DecodedImage> => { const format = sniffFormat(input.name, input.buffer) if (format === 'raw') { const decoded = await decodeRaw(input.buffer) - return { image: decoded.image, metadata: liftRawMetadata(decoded.metadata) } + return { image: decoded.image, metadata: decoded.metadata } }🤖 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 `@src/components/tools/RawTuner/decode/decode.ts` at line 113, The function liftRawMetadata currently returns its input unchanged (identity) and is unused for any transformation; either remove it and inline the metadata where it's used (replace calls to liftRawMetadata(...) with the raw metadata directly) or keep it but add a short comment above liftRawMetadata explaining it's a deliberate placeholder for future metadata mapping from DecodedRawMetadata to DecodedImageMetadata; update all call sites to match the chosen approach (references: liftRawMetadata, DecodedRawMetadata, DecodedImageMetadata).src/components/tools/RawTuner/presets/retrieve.ts (1)
50-50: ⚡ Quick winClamp and sanitise
mmrLambdabefore scoring.
topNdocuments lambda semantics in[0,1], but currently accepts invalid values (NaN, negatives, >1`) and can produce unstable ranking output.Proposed fix
- const lambda = options.mmrLambda ?? 0.7 + const rawLambda = options.mmrLambda ?? 0.7 + const lambda = Number.isFinite(rawLambda) ? Math.min(1, Math.max(0, rawLambda)) : 0.7As per coding guidelines, "Validate inputs, handle errors safely".
🤖 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 `@src/components/tools/RawTuner/presets/retrieve.ts` at line 50, The code reads options.mmrLambda into lambda without validation which allows NaN or out-of-range values; change the assignment that sets lambda (currently "lambda" from options.mmrLambda) to first validate Number.isFinite(options.mmrLambda) and then clamp it into [0,1] (e.g., use Math.max(0, Math.min(1, options.mmrLambda))) and fall back to the default 0.7 when invalid so downstream scoring/ranking functions use a stable, sanitized lambda.src/components/tools/RawTuner/ui/ToolBody.spec.tsx (2)
1-2: ⚡ Quick winUse the shared typed test utilities for this spec.
This file imports directly from
@testing-library/react; switch tosrc/test/test-utils.tsxto keep test setup and typing consistent across suites.As per coding guidelines, "Use type-safe test utilities from src/test/test-utils.tsx".
🤖 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 `@src/components/tools/RawTuner/ui/ToolBody.spec.tsx` around lines 1 - 2, The spec imports testing helpers directly from '@testing-library/react'; update ToolBody.spec.tsx to use the project’s shared typed test utilities instead by replacing imports of render, fireEvent, screen, waitFor with the equivalents exported from src/test/test-utils.tsx so the suite uses the type-safe wrappers; keep vitest imports (afterEach, beforeEach, describe, expect, it, vi) unchanged and ensure any existing references to render/fireEvent/screen/waitFor continue to work with the shared utilities.
143-149: ⚡ Quick winAvoid 2-second
waitForwindows in this suite.The explicit
timeout: 2000allowances are out of step with the suite runtime budget. Prefer tighter async assertions (or deterministic signals) that keep each test under the target threshold.As per coding guidelines, "Tests must run < 200ms".
Also applies to: 205-210
🤖 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 `@src/components/tools/RawTuner/ui/ToolBody.spec.tsx` around lines 143 - 149, Replace the long explicit wait windows by using tighter/deterministic async assertions: remove the { timeout: 2000 } options and either switch from waitFor + getAllByRole to awaiting screen.findAllByRole(...) and asserting its length (e.g., const buttons = await screen.findAllByRole('button'); expect(buttons.length).toBeGreaterThan(2)) or use waitFor with a much smaller timeout (<=200ms). Apply the same change to the similar block that covers lines 205-210 so both instances stop using the 2000ms timeout and meet the "< 200ms" test budget; update uses of waitFor, screen.getAllByRole, and any equivalent assertions accordingly.
🤖 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 `@src/components/tools/RawTuner/applier/cpu-fallback.ts`:
- Around line 146-159: applyOnCpu currently allocates a full-frame float buffer
by calling applyLinear(image, sliders) then converting to bytes; change it to
avoid that allocation by fusing the linear pass with sRGB encoding or by tiling.
Specifically, refactor so applyOnCpu no longer assigns `linear =
applyLinear(...)` but instead either (A) modify applyLinear to accept an output
Float32Array or a callback so it writes per-pixel linear values directly into
the provided buffer or encodes them immediately, or (B) implement a per-pixel
loop in applyOnCpu that invokes the same per-pixel linear computations used in
applyLinear and then calls encodeSrgb(...) and clamp01(...) into the existing
`out` Uint8ClampedArray, using `pixels`, `PIXEL_STRIDE`, and the
SliderStack/LinearImage inputs; ensure no full-frame intermediate float array is
allocated (or process in tiles) and preserve the exact per-channel math and
rounding behavior.
- Around line 25-35: The evalCurve function must guard against empty or
malformed CurvePoint arrays: first check if points is empty and return a safe
default (e.g., 0 or the provided fallback) or throw a controlled error; then
normalise the list by sorting by x and collapsing duplicate x values (e.g., keep
the last/average y for identical x) to ensure strictly increasing x; in the
interpolation loop skip or handle zero-width segments (when b.x === a.x) to
avoid division by zero and NaN; finally, proceed with the existing interpolation
logic using the cleaned points (refer to evalCurve and the CurvePoint points
array).
In `@src/components/tools/RawTuner/applier/index.ts`:
- Around line 34-52: Wrap the asynchronous probe sequence anchored at
state.initialising so any thrown error is caught and handled by falling back to
CPU; specifically, inside the async IIFE that calls requestWebGpuDevice,
webgpuSelfTest and createPipeline, add a try/catch around the probe steps and on
any error set state.decision = 'cpu', clear pipelineCache if set, and
swallow/return instead of letting the error escape to apply(); ensure
state.initialising is still awaited and nulled as currently done so apply()
observes the safe CPU decision.
In `@src/components/tools/RawTuner/applier/webgpu/pipeline.ts`:
- Around line 105-170: Wrap the buffer allocation + usage and readback (the code
that creates inputBuffer, outputBuffer, sliderBuffer, curveBuffer, stagingBuffer
and calls device.queue.writeBuffer, device.createBindGroup, encoder/submit,
stagingBuffer.mapAsync and the conversion of linear->out) in a try/finally so
GPU resources are always released; in finally unmap the stagingBuffer only if it
is mapped and call destroy() on inputBuffer, outputBuffer, sliderBuffer,
curveBuffer and stagingBuffer; also validate buffer sizes/inputs (pixels,
inputBytes, outputBytes, curveBytes, SLIDER_UNIFORM_BYTES) before creating
buffers to avoid creating resources for invalid inputs.
In `@src/components/tools/RawTuner/applier/webgpu/self-test.spec.ts`:
- Around line 14-22: The helper buildBaselineFakeDevice contains a no-op
warm-up: it constructs `probe` and awaits `webgpuSelfTest(probe as unknown as
GPUDevice)` but discards the result; remove the dead work by deleting the
`probe` creation and the `await webgpuSelfTest(...)` call (and any related
comment about seeding from that call), leaving buildBaselineFakeDevice to simply
return `new FakeDevice()`; apply the same removal to the duplicate warm-up at
the other occurrence referenced (the similar `webgpuSelfTest` call on lines
84-85).
In `@src/components/tools/RawTuner/clip/raw-image-bridge.ts`:
- Around line 18-28: linearToSrgbRgb currently drops alpha but leaves raw RGB,
causing transparent pixels to skew embeddings; update linearToSrgbRgb to
composite/premultiply alpha before encoding by reading the alpha channel (at
image.data[src + 3] using PIXEL_STRIDE), compute premultiplied RGB = srcRGB *
alpha, or composite over a chosen background color (e.g., white or configurable
background value) using standard over operator (outRGB = srcRGB * alpha + bgRGB
* (1 - alpha)), then pass the composited linear RGB values into encodeSrgb and
scale to 0–255; alternatively, if you prefer ignoring fully transparent pixels,
detect alpha === 0 and set destination RGB to the background color or skip/zero
them so they don't influence CLIP.
In `@src/components/tools/RawTuner/domain/downsample.ts`:
- Around line 12-18: downsample currently computes scale using maxSide without
validation, so invalid values (<=0, NaN, Infinity) produce surprising output;
update the downsample function to validate maxSide (ensure it's a finite number
> 0) before computing scale and throw a clear RangeError or TypeError when
invalid, and only compute scale/targetW/targetH after that validation; reference
the downsample function and variables maxSide/scale/targetW/targetH when making
the change.
In `@src/components/tools/RawTuner/heuristics/analyse.ts`:
- Around line 36-67: Add JSDoc comments for the exported helper functions
meanChannels, wbTemp, and wbTint: for meanChannels describe that it computes
per-channel mean of a LinearImage (document the image parameter, expected
LinearImage.data layout and PIXEL_STRIDE usage, and that it returns a readonly
tuple [r,g,b] of averages); for wbTemp document parameters r and b, explain it
returns a normalized warm/cool offset in range [-1,1] and that it returns 0 for
r+b===0; for wbTint document parameters r,g,b, explain it returns a normalized
green/magenta offset in range [-1,1] computed against (r+b)/2 and that it
returns 0 when denominator is 0; include `@param` and `@returns` tags and a short
one-line summary for each function.
In `@src/components/tools/RawTuner/index.spec.tsx`:
- Line 1: Update the test import in RawTuner's spec to use the project's
centralized test utilities: replace the direct import of render and screen from
'@testing-library/react' in src/components/tools/RawTuner/index.spec.tsx with an
import from the central test helper (use '../../../test/test-utils') so the test
uses the project's shared providers and utilities for render and screen.
In `@src/components/tools/RawTuner/index.tsx`:
- Around line 8-10: Update the user-facing copy in the RawTuner component: find
the paragraph in src/components/tools/RawTuner/index.tsx that currently reads
"Everything runs in your browser on your GPU. Nothing is uploaded." and change
it to a softened claim such as "Everything runs in your browser, using your GPU
when available. Nothing is uploaded." so the text reflects the CPU fallback;
edit the JSX text node inside the RawTuner component's <p> element accordingly.
In `@src/components/tools/RawTuner/presets/index.spec.ts`:
- Around line 32-37: The test currently hardcodes PRESETS[5] which breaks when
the preset bank size changes; update the test in the 'produces sensible top-1
retrievals for a description-matching query' spec so it first asserts PRESETS is
non-empty (e.g., expect(PRESETS.length).toBeGreaterThan(0)) and then choose a
guaranteed-existing element such as PRESETS[0] or PRESETS[PRESETS.length - 1] as
the reference used with topN(reference.embedding, PRESETS, 1, { mmrLambda: 1 })
and keep the final expect(result[0].name).toBe(reference.name).
In `@src/components/tools/RawTuner/ui/DropZone.tsx`:
- Around line 3-7: Add JSDoc comments for the Props interface and the DropZone
component: document the Props interface (interface Props) with a short
description and a `@property` for onFile describing the callback signature and
when it is called, and add a JSDoc block above the DropZone function describing
the component purpose, its parameter ({ onFile }: Props) and the
React.JSX.Element return value (use `@param` and `@returns`). Keep comments concise
and follow existing project JSDoc style.
In `@src/components/tools/RawTuner/ui/ExportPanel.tsx`:
- Around line 3-11: Add JSDoc comments for the ExportPanel component and its
Props interface: document the Props interface (describe each prop: disabled,
onExportJpeg(quality: number), onExportXmp()), add a top-level JSDoc for the
ExportPanel function describing its purpose and behavior, and annotate
DEFAULT_QUALITY with a short comment explaining the default JPEG quality; place
comments immediately above the interface, DEFAULT_QUALITY, and the ExportPanel
declaration to follow project conventions.
In `@src/components/tools/RawTuner/ui/PresetGrid.spec.tsx`:
- Line 1: The test imports fireEvent, render, and screen directly from
'@testing-library/react' which bypasses the project's centralized test
utilities; change the import to use the repository's test-utils re-exports (the
module that re-exports RTL utilities) so the test uses the type-safe
wrappers—replace the import of fireEvent, render, screen with the same named
imports from the central test-utils module (the file that re-exports these
utilities) so components like PresetGrid.spec.tsx use the standardized test
setup.
In `@src/components/tools/RawTuner/ui/ToolBody.tsx`:
- Around line 62-66: The async preview/suggestion callbacks (e.g.,
renderPreview) can return out-of-order results and overwrite newer state; add a
per-invocation request id guard: maintain a component-scoped incremental ref
(e.g., previewRequestIdRef), increment it at the start of renderPreview, capture
the current id in the async closure, and only call setPreviewBytes and
setApplierDecision(getApplierDecision()) if the captured id matches the ref's
current value; apply the same request-id pattern to the other async handlers
that call apply/getApplierDecision and set state so stale async completions are
ignored.
In `@tests/post-build/raw-tuner.isolation.test.ts`:
- Around line 42-75: The test only checks main.*.js and can miss leaks in shared
JS or run with zero mainChunks; assert mainChunks is non-empty (e.g.,
expect(mainChunks.length).toBeGreaterThan(0)) and fail early if empty, and
extend the leak scan to all JS assets referenced by non-raw-tuner HTML pages:
collect JS srcs from the HTML files found in htmls (or glob assets/js/**/*.js
excluding tools/raw-tuner), read each referenced/shared JS file and scan for
RAW_TUNER_MARKERS and HEAVY_DEP_MARKERS (use the same leaks array logic),
keeping the existing assertions for both marker sets.
---
Nitpick comments:
In `@plans/raw-tuner.md`:
- Around line 24-67: The fenced code block in plans/raw-tuner.md containing the
directory tree (starts with "RawTuner/") lacks a language tag; update the
opening fence from ``` to ```text (or another appropriate language like `bash`
or `plain`) so markdownlint and syntax highlighters recognize it, i.e., edit the
directory-structure code block in plans/raw-tuner.md to include the language
identifier.
In `@src/components/tools/RawTuner/applier/webgpu/fake-device.ts`:
- Around line 31-214: The classes in this test fake module (FakeBuffer,
FakeQueue, FakeComputePass, FakeCommandEncoder, FakeDevice) lack class-level
JSDoc explaining their responsibilities and important behaviors (e.g.
FakeBuffer.resolveReadback), so add brief JSDoc comments above each class
declaration describing its role, the meaning of key fields (mappedReadback,
resolveReadback, isMapped, writes, copies, dispatched), and any lifecycle notes
(e.g. mapAsync->getMappedRange->unmap, how stagingProducer/stagingBytes are
applied in FakeDevice.createBuffer); reference the exact class names
(FakeBuffer, FakeQueue, FakeComputePass, FakeCommandEncoder, FakeDevice) and the
resolveReadback/stagingProducer interaction so reviewers can locate and verify
the docs quickly.
In `@src/components/tools/RawTuner/decode/decode-jpeg.ts`:
- Around line 48-51: The error message in decode-jpeg.ts builds a string using
the Unicode multiplication sign (U+00D7); update the throw in the block that
compares rgba.length to expected (using variables rgba, expected, width, height,
PIXEL_STRIDE) to use a plain lowercase 'x' between width and height (e.g.
`${width}x${height}x4`) so the message is portable across terminals and log
viewers.
In `@src/components/tools/RawTuner/decode/decode.ts`:
- Line 113: The function liftRawMetadata currently returns its input unchanged
(identity) and is unused for any transformation; either remove it and inline the
metadata where it's used (replace calls to liftRawMetadata(...) with the raw
metadata directly) or keep it but add a short comment above liftRawMetadata
explaining it's a deliberate placeholder for future metadata mapping from
DecodedRawMetadata to DecodedImageMetadata; update all call sites to match the
chosen approach (references: liftRawMetadata, DecodedRawMetadata,
DecodedImageMetadata).
In `@src/components/tools/RawTuner/domain/downsample.spec.ts`:
- Around line 16-21: The test uses a very large fixture (createLinearImage(2048,
2048)) which makes downsample(image, 1024) slow; shrink the image to a much
smaller size that still exercises behavior (e.g., 256x256 or 512x512) and adjust
pixel fills that use PIXEL_STRIDE accordingly so the test stays under ~200ms;
apply the same reduction to the other large fixture blocks referenced (around
the other downsample usages at lines 48-63) and keep calls to createLinearImage,
downsample, and any loops that set image.data consistent with the new
dimensions.
In `@src/components/tools/RawTuner/domain/histogram.ts`:
- Around line 102-112: Add a brief inline comment above the pixel loop in
histogram.ts explaining the asymmetric clipping logic: that the highlight check
(uses r >= highThreshold || g >= highThreshold || b >= highThreshold) treats
per-channel clipping because highlights can clip in individual channels, whereas
the shadow check (uses r <= lowThreshold && g <= lowThreshold && b <=
lowThreshold) requires all channels to be low to count as shadow clipping;
reference the loop variables (data, PIXEL_STRIDE, highThreshold, lowThreshold,
high, low) so future maintainers understand the intent.
In `@src/components/tools/RawTuner/export/encode-jpeg.ts`:
- Around line 47-56: The code defensively sets canvas.width and canvas.height
after creating the canvas via factory/defaultCanvasFactory in encodeJpeg, which
is redundant if the factory always initializes size; either remove the explicit
assignments (canvas.width, canvas.height) and rely on the factory, or document
the factory contract by adding a comment above
defaultCanvasFactory/options.canvasFactory that implementations must set
width/height; update encodeJpeg to either delete those two lines or add a brief
comment referencing factory/defaultCanvasFactory to make the expectation
explicit.
In `@src/components/tools/RawTuner/presets/retrieve.ts`:
- Line 50: The code reads options.mmrLambda into lambda without validation which
allows NaN or out-of-range values; change the assignment that sets lambda
(currently "lambda" from options.mmrLambda) to first validate
Number.isFinite(options.mmrLambda) and then clamp it into [0,1] (e.g., use
Math.max(0, Math.min(1, options.mmrLambda))) and fall back to the default 0.7
when invalid so downstream scoring/ranking functions use a stable, sanitized
lambda.
In `@src/components/tools/RawTuner/storage/opfs-cache.ts`:
- Around line 140-145: Add a short inline comment above the cast where
response.body is forced to ReadableStream<Uint8Array> in the block that calls
consumeStreaming; explain that the Fetch API's typings are looser than the
runtime in this environment and that we guarantee response.body is a
ReadableStream<Uint8Array> (e.g., produced by our fetch wrapper or environment)
so the cast is safe—reference the response.body cast and the call to
consumeStreaming to locate the spot.
In `@src/components/tools/RawTuner/ui/DropZone.tsx`:
- Around line 12-15: handleFiles currently trusts the input's accept attribute
and passes the first file straight to onFile; add runtime validation inside
handleFiles to verify the file's MIME type and/or extension against an explicit
SUPPORTED_FILE_TYPES (or SUPPORTED_EXTENSIONS) list (or derive allowed types
from the input's accept prop) and reject any file that doesn't match (e.g., call
an onError callback, show a user message, or return early) before calling
onFile; update handleFiles and ensure any helper constant (e.g.,
SUPPORTED_FILE_TYPES) and error handling path are implemented so unsupported
files are not processed.
In `@src/components/tools/RawTuner/ui/ToolBody.spec.tsx`:
- Around line 1-2: The spec imports testing helpers directly from
'@testing-library/react'; update ToolBody.spec.tsx to use the project’s shared
typed test utilities instead by replacing imports of render, fireEvent, screen,
waitFor with the equivalents exported from src/test/test-utils.tsx so the suite
uses the type-safe wrappers; keep vitest imports (afterEach, beforeEach,
describe, expect, it, vi) unchanged and ensure any existing references to
render/fireEvent/screen/waitFor continue to work with the shared utilities.
- Around line 143-149: Replace the long explicit wait windows by using
tighter/deterministic async assertions: remove the { timeout: 2000 } options and
either switch from waitFor + getAllByRole to awaiting screen.findAllByRole(...)
and asserting its length (e.g., const buttons = await
screen.findAllByRole('button'); expect(buttons.length).toBeGreaterThan(2)) or
use waitFor with a much smaller timeout (<=200ms). Apply the same change to the
similar block that covers lines 205-210 so both instances stop using the 2000ms
timeout and meet the "< 200ms" test budget; update uses of waitFor,
screen.getAllByRole, and any equivalent assertions accordingly.
🪄 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: bdf2d70a-978e-4464-88ca-d490fc51ff7b
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (73)
package.jsonplans/raw-tuner.mdsrc/components/tools/RawTuner/applier/cpu-fallback.spec.tssrc/components/tools/RawTuner/applier/cpu-fallback.tssrc/components/tools/RawTuner/applier/index.spec.tssrc/components/tools/RawTuner/applier/index.tssrc/components/tools/RawTuner/applier/webgpu/fake-device.tssrc/components/tools/RawTuner/applier/webgpu/pipeline.spec.tssrc/components/tools/RawTuner/applier/webgpu/pipeline.tssrc/components/tools/RawTuner/applier/webgpu/self-test.spec.tssrc/components/tools/RawTuner/applier/webgpu/self-test.tssrc/components/tools/RawTuner/applier/webgpu/shader.tssrc/components/tools/RawTuner/clip/load-clip.spec.tssrc/components/tools/RawTuner/clip/load-clip.tssrc/components/tools/RawTuner/clip/loading-progress.spec.tssrc/components/tools/RawTuner/clip/loading-progress.tssrc/components/tools/RawTuner/clip/raw-image-bridge.spec.tssrc/components/tools/RawTuner/clip/raw-image-bridge.tssrc/components/tools/RawTuner/decode/decode-jpeg.spec.tssrc/components/tools/RawTuner/decode/decode-jpeg.tssrc/components/tools/RawTuner/decode/decode-raw.spec.tssrc/components/tools/RawTuner/decode/decode-raw.tssrc/components/tools/RawTuner/decode/decode.spec.tssrc/components/tools/RawTuner/decode/decode.tssrc/components/tools/RawTuner/domain/downsample.spec.tssrc/components/tools/RawTuner/domain/downsample.tssrc/components/tools/RawTuner/domain/histogram.spec.tssrc/components/tools/RawTuner/domain/histogram.tssrc/components/tools/RawTuner/domain/linear-image.spec.tssrc/components/tools/RawTuner/domain/linear-image.tssrc/components/tools/RawTuner/domain/slider-stack.spec.tssrc/components/tools/RawTuner/domain/slider-stack.tssrc/components/tools/RawTuner/export/encode-jpeg.spec.tssrc/components/tools/RawTuner/export/encode-jpeg.tssrc/components/tools/RawTuner/export/write-xmp.spec.tssrc/components/tools/RawTuner/export/write-xmp.tssrc/components/tools/RawTuner/heuristics/analyse.spec.tssrc/components/tools/RawTuner/heuristics/analyse.tssrc/components/tools/RawTuner/heuristics/auto-tune.spec.tssrc/components/tools/RawTuner/heuristics/auto-tune.tssrc/components/tools/RawTuner/heuristics/round-trip.spec.tssrc/components/tools/RawTuner/index.spec.tsxsrc/components/tools/RawTuner/index.tsxsrc/components/tools/RawTuner/presets/build/embed-presets.spec.tssrc/components/tools/RawTuner/presets/build/embed-presets.tssrc/components/tools/RawTuner/presets/index.spec.tssrc/components/tools/RawTuner/presets/index.tssrc/components/tools/RawTuner/presets/presets.jsonsrc/components/tools/RawTuner/presets/presets.source.tssrc/components/tools/RawTuner/presets/retrieve.spec.tssrc/components/tools/RawTuner/presets/retrieve.tssrc/components/tools/RawTuner/presets/types.tssrc/components/tools/RawTuner/storage/opfs-cache.spec.tssrc/components/tools/RawTuner/storage/opfs-cache.tssrc/components/tools/RawTuner/ui/DropZone.spec.tsxsrc/components/tools/RawTuner/ui/DropZone.tsxsrc/components/tools/RawTuner/ui/ExportPanel.spec.tsxsrc/components/tools/RawTuner/ui/ExportPanel.tsxsrc/components/tools/RawTuner/ui/HistogramView.spec.tsxsrc/components/tools/RawTuner/ui/HistogramView.tsxsrc/components/tools/RawTuner/ui/PresetGrid.spec.tsxsrc/components/tools/RawTuner/ui/PresetGrid.tsxsrc/components/tools/RawTuner/ui/SliderStack.spec.tsxsrc/components/tools/RawTuner/ui/SliderStack.tsxsrc/components/tools/RawTuner/ui/ToolBody.spec.tsxsrc/components/tools/RawTuner/ui/ToolBody.tsxsrc/filetypes.d.tssrc/pages/tools/index.tsxsrc/pages/tools/raw-tuner/index.tsxsrc/test/mocks/tool-page.tsxtests/post-build/raw-tuner.isolation.test.tstsconfig.jsonvitest.config.mts
Comment on lines
+25
to
+35
| const evalCurve = (value: number, points: readonly CurvePoint[]): number => { | ||
| if (value <= points[0].x) return points[0].y | ||
| for (let i = 0; i < points.length - 1; i++) { | ||
| const a = points[i] | ||
| const b = points[i + 1] | ||
| if (value <= b.x) { | ||
| const t = (value - a.x) / (b.x - a.x) | ||
| return a.y + (b.y - a.y) * t | ||
| } | ||
| } | ||
| return points[points.length - 1].y |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Fail safely on degenerate tone curves.
evalCurve assumes a non-empty, strictly increasing point list. An empty array throws on points[0], and duplicate or reversed x values can turn the interpolation into NaN, which then poisons the whole render/export buffer. Please normalise or reject malformed curves before interpolating.
Possible local hardening
const evalCurve = (value: number, points: readonly CurvePoint[]): number => {
+ if (points.length < 2) return value
if (value <= points[0].x) return points[0].y
for (let i = 0; i < points.length - 1; i++) {
const a = points[i]
const b = points[i + 1]
+ if (b.x <= a.x) continue
if (value <= b.x) {
const t = (value - a.x) / (b.x - a.x)
return a.y + (b.y - a.y) * t
}
}
return points[points.length - 1].y
}As per coding guidelines, "Validate inputs, handle errors safely".
📝 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
| const evalCurve = (value: number, points: readonly CurvePoint[]): number => { | |
| if (value <= points[0].x) return points[0].y | |
| for (let i = 0; i < points.length - 1; i++) { | |
| const a = points[i] | |
| const b = points[i + 1] | |
| if (value <= b.x) { | |
| const t = (value - a.x) / (b.x - a.x) | |
| return a.y + (b.y - a.y) * t | |
| } | |
| } | |
| return points[points.length - 1].y | |
| const evalCurve = (value: number, points: readonly CurvePoint[]): number => { | |
| if (points.length < 2) return value | |
| if (value <= points[0].x) return points[0].y | |
| for (let i = 0; i < points.length - 1; i++) { | |
| const a = points[i] | |
| const b = points[i + 1] | |
| if (b.x <= a.x) continue | |
| if (value <= b.x) { | |
| const t = (value - a.x) / (b.x - a.x) | |
| return a.y + (b.y - a.y) * t | |
| } | |
| } | |
| return points[points.length - 1].y | |
| } |
🤖 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 `@src/components/tools/RawTuner/applier/cpu-fallback.ts` around lines 25 - 35,
The evalCurve function must guard against empty or malformed CurvePoint arrays:
first check if points is empty and return a safe default (e.g., 0 or the
provided fallback) or throw a controlled error; then normalise the list by
sorting by x and collapsing duplicate x values (e.g., keep the last/average y
for identical x) to ensure strictly increasing x; in the interpolation loop skip
or handle zero-width segments (when b.x === a.x) to avoid division by zero and
NaN; finally, proceed with the existing interpolation logic using the cleaned
points (refer to evalCurve and the CurvePoint points array).
Comment on lines
+146
to
+159
| export const applyOnCpu = (image: LinearImage, sliders: SliderStack): Uint8ClampedArray => { | ||
| const linear = applyLinear(image, sliders) | ||
| const out = new Uint8ClampedArray(linear.length) | ||
| const pixels = image.width * image.height | ||
|
|
||
| for (let i = 0; i < pixels; i++) { | ||
| const idx = i * PIXEL_STRIDE | ||
| out[idx + 0] = Math.round(encodeSrgb(linear[idx + 0]) * 255) | ||
| out[idx + 1] = Math.round(encodeSrgb(linear[idx + 1]) * 255) | ||
| out[idx + 2] = Math.round(encodeSrgb(linear[idx + 2]) * 255) | ||
| out[idx + 3] = Math.round(clamp01(linear[idx + 3]) * 255) | ||
| } | ||
|
|
||
| return out |
Contributor
There was a problem hiding this comment.
Avoid the extra full-frame float buffer in the CPU path.
applyOnCpu allocates linear = applyLinear(...) and then allocates the byte output. On a 24 MP image that is roughly another 384 MB transient allocation before the 96 MB RGBA buffer, which is a real tab-killer on the same non-WebGPU devices that will hit this fallback. Fusing the linear pass with sRGB encoding, or processing in tiles, would keep the CPU path usable for full-resolution exports.
🤖 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 `@src/components/tools/RawTuner/applier/cpu-fallback.ts` around lines 146 -
159, applyOnCpu currently allocates a full-frame float buffer by calling
applyLinear(image, sliders) then converting to bytes; change it to avoid that
allocation by fusing the linear pass with sRGB encoding or by tiling.
Specifically, refactor so applyOnCpu no longer assigns `linear =
applyLinear(...)` but instead either (A) modify applyLinear to accept an output
Float32Array or a callback so it writes per-pixel linear values directly into
the provided buffer or encodes them immediately, or (B) implement a per-pixel
loop in applyOnCpu that invokes the same per-pixel linear computations used in
applyLinear and then calls encodeSrgb(...) and clamp01(...) into the existing
`out` Uint8ClampedArray, using `pixels`, `PIXEL_STRIDE`, and the
SliderStack/LinearImage inputs; ensure no full-frame intermediate float array is
allocated (or process in tiles) and preserve the exact per-channel math and
rounding behavior.
Comment on lines
+34
to
+52
| state.initialising = (async () => { | ||
| const device = await requestWebGpuDevice() | ||
| if (!device) { | ||
| state.decision = 'cpu' | ||
| return | ||
| } | ||
| const ok = await webgpuSelfTest(device) | ||
| if (!ok) { | ||
| state.decision = 'cpu' | ||
| return | ||
| } | ||
| state.decision = 'gpu' | ||
| state.pipelineCache = { device, pipeline: createPipeline(device) } | ||
| })() | ||
| try { | ||
| await state.initialising | ||
| } finally { | ||
| state.initialising = null | ||
| } |
Contributor
There was a problem hiding this comment.
Fail closed to CPU if initialisation throws.
A thrown error during probing (requestWebGpuDevice/webgpuSelfTest/createPipeline) currently bubbles out of apply() instead of safely selecting CPU.
🛠️ Suggested hardening
- state.initialising = (async () => {
- const device = await requestWebGpuDevice()
- if (!device) {
- state.decision = 'cpu'
- return
- }
- const ok = await webgpuSelfTest(device)
- if (!ok) {
- state.decision = 'cpu'
- return
- }
- state.decision = 'gpu'
- state.pipelineCache = { device, pipeline: createPipeline(device) }
- })()
+ state.initialising = (async () => {
+ try {
+ const device = await requestWebGpuDevice()
+ if (!device) {
+ state.decision = 'cpu'
+ return
+ }
+ const ok = await webgpuSelfTest(device)
+ if (!ok) {
+ state.decision = 'cpu'
+ return
+ }
+ state.decision = 'gpu'
+ state.pipelineCache = { device, pipeline: createPipeline(device) }
+ } catch {
+ state.decision = 'cpu'
+ state.pipelineCache = null
+ }
+ })()As per coding guidelines, “Validate inputs, handle errors safely”.
📝 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
| state.initialising = (async () => { | |
| const device = await requestWebGpuDevice() | |
| if (!device) { | |
| state.decision = 'cpu' | |
| return | |
| } | |
| const ok = await webgpuSelfTest(device) | |
| if (!ok) { | |
| state.decision = 'cpu' | |
| return | |
| } | |
| state.decision = 'gpu' | |
| state.pipelineCache = { device, pipeline: createPipeline(device) } | |
| })() | |
| try { | |
| await state.initialising | |
| } finally { | |
| state.initialising = null | |
| } | |
| state.initialising = (async () => { | |
| try { | |
| const device = await requestWebGpuDevice() | |
| if (!device) { | |
| state.decision = 'cpu' | |
| return | |
| } | |
| const ok = await webgpuSelfTest(device) | |
| if (!ok) { | |
| state.decision = 'cpu' | |
| return | |
| } | |
| state.decision = 'gpu' | |
| state.pipelineCache = { device, pipeline: createPipeline(device) } | |
| } catch { | |
| state.decision = 'cpu' | |
| state.pipelineCache = null | |
| } | |
| })() | |
| try { | |
| await state.initialising | |
| } finally { | |
| state.initialising = null | |
| } |
🤖 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 `@src/components/tools/RawTuner/applier/index.ts` around lines 34 - 52, Wrap
the asynchronous probe sequence anchored at state.initialising so any thrown
error is caught and handled by falling back to CPU; specifically, inside the
async IIFE that calls requestWebGpuDevice, webgpuSelfTest and createPipeline,
add a try/catch around the probe steps and on any error set state.decision =
'cpu', clear pipelineCache if set, and swallow/return instead of letting the
error escape to apply(); ensure state.initialising is still awaited and nulled
as currently done so apply() observes the safe CPU decision.
Comment on lines
+105
to
+170
| const inputBuffer = device.createBuffer({ | ||
| label: 'raw-tuner.input', | ||
| size: inputBytes, | ||
| usage: USAGE_STORAGE | USAGE_COPY_DST, | ||
| }) | ||
| const outputBuffer = device.createBuffer({ | ||
| label: 'raw-tuner.output', | ||
| size: outputBytes, | ||
| usage: USAGE_STORAGE | USAGE_COPY_SRC, | ||
| }) | ||
| const sliderBuffer = device.createBuffer({ | ||
| label: 'raw-tuner.sliders', | ||
| size: SLIDER_UNIFORM_BYTES, | ||
| usage: USAGE_UNIFORM | USAGE_COPY_DST, | ||
| }) | ||
| const curveBuffer = device.createBuffer({ | ||
| label: 'raw-tuner.curve', | ||
| size: curveBytes, | ||
| usage: USAGE_STORAGE | USAGE_COPY_DST, | ||
| }) | ||
| const stagingBuffer = device.createBuffer({ | ||
| label: 'raw-tuner.staging', | ||
| size: outputBytes, | ||
| usage: USAGE_MAP_READ | USAGE_COPY_DST, | ||
| }) | ||
|
|
||
| device.queue.writeBuffer(inputBuffer, 0, image.data) | ||
| device.queue.writeBuffer(sliderBuffer, 0, writeSliderUniform(sliders)) | ||
| device.queue.writeBuffer(curveBuffer, 0, writeCurveBuffer(sliders)) | ||
|
|
||
| const bindGroup = device.createBindGroup({ | ||
| label: 'raw-tuner.binding', | ||
| layout: pipeline.getBindGroupLayout(0), | ||
| entries: [ | ||
| { binding: 0, resource: { buffer: inputBuffer } }, | ||
| { binding: 1, resource: { buffer: outputBuffer } }, | ||
| { binding: 2, resource: { buffer: sliderBuffer } }, | ||
| { binding: 3, resource: { buffer: curveBuffer } }, | ||
| ], | ||
| }) | ||
|
|
||
| const encoder = device.createCommandEncoder({ label: 'raw-tuner.encoder' }) | ||
| const pass = encoder.beginComputePass({ label: 'raw-tuner.pass' }) | ||
| pass.setPipeline(pipeline) | ||
| pass.setBindGroup(0, bindGroup) | ||
| pass.dispatchWorkgroups(Math.ceil(pixels / WORKGROUP_SIZE)) | ||
| pass.end() | ||
| encoder.copyBufferToBuffer(outputBuffer, 0, stagingBuffer, 0, outputBytes) | ||
| device.queue.submit([encoder.finish()]) | ||
|
|
||
| await stagingBuffer.mapAsync(MAP_MODE_READ) | ||
| const linear = new Float32Array(stagingBuffer.getMappedRange().slice(0)) | ||
| stagingBuffer.unmap() | ||
|
|
||
| // The shader writes sRGB-encoded values in [0,1]. Convert to bytes with the | ||
| // same rounding the CPU path uses so outputs match across paths. | ||
| const out = new Uint8ClampedArray(linear.length) | ||
| for (let i = 0; i < linear.length; i++) { | ||
| out[i] = Math.round(Math.max(0, Math.min(1, linear[i])) * 255) | ||
| } | ||
|
|
||
| inputBuffer.destroy() | ||
| outputBuffer.destroy() | ||
| sliderBuffer.destroy() | ||
| curveBuffer.destroy() | ||
| stagingBuffer.destroy() |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "pipeline.ts" -path "*webgpu*" | head -5Repository: webbertakken/takken.io
Length of output: 125
🏁 Script executed:
cat -n src/components/tools/RawTuner/applier/webgpu/pipeline.ts | sed -n '1,50p'Repository: webbertakken/takken.io
Length of output: 2230
🏁 Script executed:
cat -n src/components/tools/RawTuner/applier/webgpu/pipeline.ts | sed -n '100,180p'Repository: webbertakken/takken.io
Length of output: 3060
🏁 Script executed:
cat -n src/components/tools/RawTuner/applier/webgpu/pipeline.ts | sed -n '70,180p'Repository: webbertakken/takken.io
Length of output: 4330
Release GPU buffers on every failure path.
Any throw after the buffers are created — writeBuffer, createBindGroup, submit, mapAsync, or the readback conversion — skips the destroy() calls (lines 166–170) and leaks GPU memory. This path is used for previews and self-tests, so retries will compound the leak. Wrap the allocation and readback section in try/finally and unmap conditionally before destroying.
Suggested structure
- device.queue.writeBuffer(inputBuffer, 0, image.data)
- device.queue.writeBuffer(sliderBuffer, 0, writeSliderUniform(sliders))
- device.queue.writeBuffer(curveBuffer, 0, writeCurveBuffer(sliders))
-
- const bindGroup = device.createBindGroup({
- label: 'raw-tuner.binding',
- layout: pipeline.getBindGroupLayout(0),
- entries: [
- { binding: 0, resource: { buffer: inputBuffer } },
- { binding: 1, resource: { buffer: outputBuffer } },
- { binding: 2, resource: { buffer: sliderBuffer } },
- { binding: 3, resource: { buffer: curveBuffer } },
- ],
- })
-
- const encoder = device.createCommandEncoder({ label: 'raw-tuner.encoder' })
- const pass = encoder.beginComputePass({ label: 'raw-tuner.pass' })
- pass.setPipeline(pipeline)
- pass.setBindGroup(0, bindGroup)
- pass.dispatchWorkgroups(Math.ceil(pixels / WORKGROUP_SIZE))
- pass.end()
- encoder.copyBufferToBuffer(outputBuffer, 0, stagingBuffer, 0, outputBytes)
- device.queue.submit([encoder.finish()])
-
- await stagingBuffer.mapAsync(MAP_MODE_READ)
- const linear = new Float32Array(stagingBuffer.getMappedRange().slice(0))
- stagingBuffer.unmap()
-
- const out = new Uint8ClampedArray(linear.length)
- for (let i = 0; i < linear.length; i++) {
- out[i] = Math.round(Math.max(0, Math.min(1, linear[i])) * 255)
- }
-
- inputBuffer.destroy()
- outputBuffer.destroy()
- sliderBuffer.destroy()
- curveBuffer.destroy()
- stagingBuffer.destroy()
- return out
+ try {
+ device.queue.writeBuffer(inputBuffer, 0, image.data)
+ device.queue.writeBuffer(sliderBuffer, 0, writeSliderUniform(sliders))
+ device.queue.writeBuffer(curveBuffer, 0, writeCurveBuffer(sliders))
+
+ const bindGroup = device.createBindGroup({
+ label: 'raw-tuner.binding',
+ layout: pipeline.getBindGroupLayout(0),
+ entries: [
+ { binding: 0, resource: { buffer: inputBuffer } },
+ { binding: 1, resource: { buffer: outputBuffer } },
+ { binding: 2, resource: { buffer: sliderBuffer } },
+ { binding: 3, resource: { buffer: curveBuffer } },
+ ],
+ })
+
+ const encoder = device.createCommandEncoder({ label: 'raw-tuner.encoder' })
+ const pass = encoder.beginComputePass({ label: 'raw-tuner.pass' })
+ pass.setPipeline(pipeline)
+ pass.setBindGroup(0, bindGroup)
+ pass.dispatchWorkgroups(Math.ceil(pixels / WORKGROUP_SIZE))
+ pass.end()
+ encoder.copyBufferToBuffer(outputBuffer, 0, stagingBuffer, 0, outputBytes)
+ device.queue.submit([encoder.finish()])
+
+ await stagingBuffer.mapAsync(MAP_MODE_READ)
+ const linear = new Float32Array(stagingBuffer.getMappedRange().slice(0))
+ stagingBuffer.unmap()
+
+ const out = new Uint8ClampedArray(linear.length)
+ for (let i = 0; i < linear.length; i++) {
+ out[i] = Math.round(Math.max(0, Math.min(1, linear[i])) * 255)
+ }
+ return out
+ } finally {
+ if (stagingBuffer.mapState === 'mapped') stagingBuffer.unmap()
+ inputBuffer.destroy()
+ outputBuffer.destroy()
+ sliderBuffer.destroy()
+ curveBuffer.destroy()
+ stagingBuffer.destroy()
+ }Per coding guidelines, validate inputs and handle errors safely.
🤖 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 `@src/components/tools/RawTuner/applier/webgpu/pipeline.ts` around lines 105 -
170, Wrap the buffer allocation + usage and readback (the code that creates
inputBuffer, outputBuffer, sliderBuffer, curveBuffer, stagingBuffer and calls
device.queue.writeBuffer, device.createBindGroup, encoder/submit,
stagingBuffer.mapAsync and the conversion of linear->out) in a try/finally so
GPU resources are always released; in finally unmap the stagingBuffer only if it
is mapped and call destroy() on inputBuffer, outputBuffer, sliderBuffer,
curveBuffer and stagingBuffer; also validate buffer sizes/inputs (pixels,
inputBytes, outputBytes, curveBytes, SLIDER_UNIFORM_BYTES) before creating
buffers to avoid creating resources for invalid inputs.
Comment on lines
+14
to
+22
| const buildBaselineFakeDevice = async (): Promise<FakeDevice> => { | ||
| // First pass: a fake with all-zero readback. We don't care about the | ||
| // returned bytes, only that we can grab the CPU baseline from a real call. | ||
| const probe = new FakeDevice({ stagingBytes: new Float32Array(4 * 4 * 4) }) | ||
| await webgpuSelfTest(probe as unknown as GPUDevice) | ||
| // The CPU baseline is independent of the device, so the easiest move is to | ||
| // re-create one and let the test seed `stagingBytes` per scenario. | ||
| return new FakeDevice() | ||
| } |
Contributor
There was a problem hiding this comment.
Remove the no-op warm-up from the tolerance test.
This helper call is currently dead work (baseline is discarded), so it adds test runtime and cognitive overhead without changing behaviour.
💡 Suggested cleanup
-const buildBaselineFakeDevice = async (): Promise<FakeDevice> => {
- // First pass: a fake with all-zero readback. We don't care about the
- // returned bytes, only that we can grab the CPU baseline from a real call.
- const probe = new FakeDevice({ stagingBytes: new Float32Array(4 * 4 * 4) })
- await webgpuSelfTest(probe as unknown as GPUDevice)
- // The CPU baseline is independent of the device, so the easiest move is to
- // re-create one and let the test seed `stagingBytes` per scenario.
- return new FakeDevice()
-}
@@
- const baseline = await buildBaselineFakeDevice()
- void baselineAs per coding guidelines, “Tests must run < 200ms”.
Also applies to: 84-85
🤖 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 `@src/components/tools/RawTuner/applier/webgpu/self-test.spec.ts` around lines
14 - 22, The helper buildBaselineFakeDevice contains a no-op warm-up: it
constructs `probe` and awaits `webgpuSelfTest(probe as unknown as GPUDevice)`
but discards the result; remove the dead work by deleting the `probe` creation
and the `await webgpuSelfTest(...)` call (and any related comment about seeding
from that call), leaving buildBaselineFakeDevice to simply return `new
FakeDevice()`; apply the same removal to the duplicate warm-up at the other
occurrence referenced (the similar `webgpuSelfTest` call on lines 84-85).
Comment on lines
+3
to
+7
| interface Props { | ||
| onFile: (file: File) => void | ||
| } | ||
|
|
||
| const DropZone = ({ onFile }: Props): React.JSX.Element => { |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Add JSDoc documentation for the component and its Props.
The component and interface lack JSDoc documentation. As per coding guidelines, code should be documented with JSDoc and light comments.
📝 Proposed JSDoc addition
+/**
+ * Props for the DropZone component.
+ */
interface Props {
+ /** Callback invoked when a file is selected or dropped. */
onFile: (file: File) => void
}
+/**
+ * A file drop zone that supports drag-and-drop, click-to-select, and keyboard
+ * activation. Accepts RAW and JPEG files.
+ */
const DropZone = ({ onFile }: Props): React.JSX.Element => {🤖 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 `@src/components/tools/RawTuner/ui/DropZone.tsx` around lines 3 - 7, Add JSDoc
comments for the Props interface and the DropZone component: document the Props
interface (interface Props) with a short description and a `@property` for onFile
describing the callback signature and when it is called, and add a JSDoc block
above the DropZone function describing the component purpose, its parameter ({
onFile }: Props) and the React.JSX.Element return value (use `@param` and
`@returns`). Keep comments concise and follow existing project JSDoc style.
Comment on lines
+3
to
+11
| interface Props { | ||
| disabled: boolean | ||
| onExportJpeg: (quality: number) => void | ||
| onExportXmp: () => void | ||
| } | ||
|
|
||
| const DEFAULT_QUALITY = 0.92 | ||
|
|
||
| const ExportPanel = ({ disabled, onExportJpeg, onExportXmp }: Props): React.JSX.Element => { |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Add JSDoc documentation for the component and its Props.
The component and interface lack JSDoc documentation. As per coding guidelines, code should be documented with JSDoc and light comments.
📝 Proposed JSDoc addition
+/**
+ * Props for the ExportPanel component.
+ */
interface Props {
+ /** Whether export buttons should be disabled. */
disabled: boolean
+ /** Callback to export JPEG with the specified quality (0-1). */
onExportJpeg: (quality: number) => void
+ /** Callback to export XMP sidecar file. */
onExportXmp: () => void
}
const DEFAULT_QUALITY = 0.92
+/**
+ * Export panel providing quality slider and JPEG/XMP export buttons.
+ * JPEG quality defaults to 0.92 (Lightroom-style "high").
+ */
const ExportPanel = ({ disabled, onExportJpeg, onExportXmp }: Props): React.JSX.Element => {🤖 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 `@src/components/tools/RawTuner/ui/ExportPanel.tsx` around lines 3 - 11, Add
JSDoc comments for the ExportPanel component and its Props interface: document
the Props interface (describe each prop: disabled, onExportJpeg(quality:
number), onExportXmp()), add a top-level JSDoc for the ExportPanel function
describing its purpose and behavior, and annotate DEFAULT_QUALITY with a short
comment explaining the default JPEG quality; place comments immediately above
the interface, DEFAULT_QUALITY, and the ExportPanel declaration to follow
project conventions.
| @@ -0,0 +1,53 @@ | |||
| import { fireEvent, render, screen } from '@testing-library/react' | |||
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate the test-utils file
fd -t f "test-utils" --type fRepository: webbertakken/takken.io
Length of output: 90
🏁 Script executed:
# Check if src/test/test-utils.tsx exists
ls -la src/test/ 2>/dev/null || echo "Directory not found"Repository: webbertakken/takken.io
Length of output: 359
🏁 Script executed:
# Find the actual test file to verify the import statement
fd "PresetGrid.spec" -t fRepository: webbertakken/takken.io
Length of output: 119
🏁 Script executed:
# Check what test-utils.tsx exports
cat -n src/test/test-utils.tsxRepository: webbertakken/takken.io
Length of output: 2941
🏁 Script executed:
# Check the actual imports in the test file
head -n 20 src/components/tools/RawTuner/ui/PresetGrid.spec.tsxRepository: webbertakken/takken.io
Length of output: 1018
🏁 Script executed:
# Check a few other test files to see if this is a pattern issue
fd "\.spec\.tsx$" -x head -n 1 {} \; | head -20Repository: webbertakken/takken.io
Length of output: 959
Use the centralised test-utils import instead of direct @testing-library/react imports.
The coding guideline requires using type-safe test utilities from src/test/test-utils.tsx. Although fireEvent, render, and screen are re-exported there (line 87), direct RTL imports bypass the intended wrapper, preventing centralised control over test setup, provider configuration, and standardisation across the repository.
Change the import from import { fireEvent, render, screen } from '@testing-library/react' to import from src/test/test-utils.tsx.
🤖 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 `@src/components/tools/RawTuner/ui/PresetGrid.spec.tsx` at line 1, The test
imports fireEvent, render, and screen directly from '@testing-library/react'
which bypasses the project's centralized test utilities; change the import to
use the repository's test-utils re-exports (the module that re-exports RTL
utilities) so the test uses the type-safe wrappers—replace the import of
fireEvent, render, screen with the same named imports from the central
test-utils module (the file that re-exports these utilities) so components like
PresetGrid.spec.tsx use the standardized test setup.
Comment on lines
+62
to
+66
| const renderPreview = useCallback(async (image: LinearImage, stack: SliderStack) => { | ||
| const bytes = await apply(image, stack) | ||
| setPreviewBytes(bytes) | ||
| setApplierDecision(getApplierDecision()) | ||
| }, []) |
Contributor
There was a problem hiding this comment.
Prevent stale async results from overwriting newer preview/suggestion state.
Preview renders and suggestion generation can complete out of order; an older job can overwrite the latest file/slider selection.
Proposed fix (request-id guard)
@@
const previewCanvasRef = useRef<HTMLCanvasElement>(null)
const encoderRef = useRef<ClipImageEncoder | null>(null)
+ const previewJobRef = useRef(0)
+ const suggestionJobRef = useRef(0)
@@
const renderPreview = useCallback(async (image: LinearImage, stack: SliderStack) => {
+ const jobId = ++previewJobRef.current
const bytes = await apply(image, stack)
+ if (jobId !== previewJobRef.current) return
setPreviewBytes(bytes)
setApplierDecision(getApplierDecision())
}, [])
@@
const handleFile = async (file: File) => {
@@
+ const suggestionJobId = ++suggestionJobRef.current
setLoaded({
@@
setSliders(baseline)
+ setSuggestions([])
setActivePreset('Auto-tuned')
await renderPreview(preview, baseline)
- void runSuggestions(preview)
+ void runSuggestions(preview, suggestionJobId)
@@
- const runSuggestions = async (image: LinearImage) => {
+ const runSuggestions = async (image: LinearImage, jobId: number) => {
@@
const embedding = await encoderRef.current.embed(image)
const top = topN(embedding, PRESETS, 5, { mmrLambda: 0.7 })
+ if (jobId !== suggestionJobRef.current) return
setSuggestions(top)Also applies to: 114-115, 124-132, 139-148, 150-156
🤖 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 `@src/components/tools/RawTuner/ui/ToolBody.tsx` around lines 62 - 66, The
async preview/suggestion callbacks (e.g., renderPreview) can return out-of-order
results and overwrite newer state; add a per-invocation request id guard:
maintain a component-scoped incremental ref (e.g., previewRequestIdRef),
increment it at the start of renderPreview, capture the current id in the async
closure, and only call setPreviewBytes and
setApplierDecision(getApplierDecision()) if the captured id matches the ref's
current value; apply the same request-id pattern to the other async handlers
that call apply/getApplierDecision and set state so stale async completions are
ignored.
Comment on lines
+42
to
+75
| it('does not include RawTuner code in the main entry chunks', async () => { | ||
| const mainChunks = await glob('assets/js/main.*.js', { cwd: buildDir, absolute: true }) | ||
| const leaks: string[] = [] | ||
|
|
||
| for (const chunk of mainChunks) { | ||
| const content = readFileSync(chunk, 'utf-8') | ||
| for (const marker of RAW_TUNER_MARKERS) { | ||
| if (content.includes(marker)) { | ||
| leaks.push(`${marker} leaked into ${chunk}; main entry should stay lean`) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| expect(leaks).toEqual([]) | ||
| }) | ||
|
|
||
| it('does not include RawTuner heavy deps in any non-raw-tuner HTML', async () => { | ||
| const htmls = await glob('**/*.html', { | ||
| cwd: buildDir, | ||
| absolute: true, | ||
| ignore: ['tools/raw-tuner/**'], | ||
| }) | ||
| const leaks: string[] = [] | ||
|
|
||
| for (const html of htmls) { | ||
| const content = readFileSync(html, 'utf-8') | ||
| for (const marker of HEAVY_DEP_MARKERS) { | ||
| if (content.includes(marker)) { | ||
| leaks.push(`${marker} inlined in ${html}; should be dynamically imported only`) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| expect(leaks).toEqual([]) |
Contributor
There was a problem hiding this comment.
Isolation test can miss bundle leaks outside main.*.js.
This currently proves only that main-entry chunks are clean. Leaks into shared non-main JS chunks can still pass undetected, and mainChunks can be empty without failing. Add a non-empty mainChunks assertion and inspect JS assets referenced by non-raw-tuner HTML pages.
Hardening sketch
it('does not include RawTuner code in the main entry chunks', async () => {
const mainChunks = await glob('assets/js/main.*.js', { cwd: buildDir, absolute: true })
+ expect(mainChunks.length).toBeGreaterThan(0)
const leaks: string[] = []
@@
})
+
+ it('does not include RawTuner markers in JS referenced by non-raw-tuner pages', async () => {
+ const htmls = await glob('**/*.html', {
+ cwd: buildDir,
+ absolute: true,
+ ignore: ['tools/raw-tuner/**'],
+ })
+ const scriptSrcRe = /<script[^>]+src="([^"]+\.js)"/g
+ const leaks: string[] = []
+
+ for (const html of htmls) {
+ const content = readFileSync(html, 'utf-8')
+ for (const match of content.matchAll(scriptSrcRe)) {
+ const src = match[1].replace(/^\//, '')
+ const jsPath = join(buildDir, src)
+ const js = readFileSync(jsPath, 'utf-8')
+ for (const marker of [...RAW_TUNER_MARKERS, ...HEAVY_DEP_MARKERS]) {
+ if (js.includes(marker)) leaks.push(`${marker} leaked via ${jsPath} (from ${html})`)
+ }
+ }
+ }
+
+ expect(leaks).toEqual([])
+ })🤖 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 `@tests/post-build/raw-tuner.isolation.test.ts` around lines 42 - 75, The test
only checks main.*.js and can miss leaks in shared JS or run with zero
mainChunks; assert mainChunks is non-empty (e.g.,
expect(mainChunks.length).toBeGreaterThan(0)) and fail early if empty, and
extend the leak scan to all JS assets referenced by non-raw-tuner HTML pages:
collect JS srcs from the HTML files found in htmls (or glob assets/js/**/*.js
excluding tools/raw-tuner), read each referenced/shared JS file and scan for
RAW_TUNER_MARKERS and HEAVY_DEP_MARKERS (use the same leaks array logic),
keeping the existing assertions for both marker sets.
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
1 participant
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.
What
A new
/tools/raw-tunerpage: drop a RAW or JPEG, get an auto-tuned preview plus suggested looks, export a JPEG and a Lightroom-compatible.xmp. Everything runs in the browser.Pipeline
libraw-wasmfor RAW,ImageDecoder/ canvas for JPEG/PNG/WebP) → linear-lightLinearImage(Float32 RGBA).OffscreenCanvas.convertToBlob) +.xmpsidecar with thecrs:namespace.Quality
src/components/tools/RawTuner/.yarn lint,yarn typecheck,yarn format,yarn test,yarn buildall clean.Bundling notes
transformers.jsloads fromcdn.jsdelivr.netat runtime via awebpackIgnore-marked dynamic import. Docusaurus's webpack would otherwise try to bundleonnxruntime-node+sharp.libraw-wasm(~361 KB chunk) lazy-imported only when the tool decodes a RAW.Pre-existing infra fix
jsdom@^26becausejsdom@29(Dec 2025) migrated to@exodus/byteswhich is ESM-only and breaks vitest's CJS config loader. Renamedvitest.config.ts→vitest.config.mtsso the config loads via ESM.Plan
Full checkboxed plan with phases 0–9 in
plans/raw-tuner.md.Manual smoke (operator)
Drop a real CR2 / NEF / DNG, verify the auto-tune is plausible, check the JPEG export looks right, round-trip the
.xmpinto Lightroom.Summary by CodeRabbit
Release Notes