Skip to content

🔍 Duplicate Code Detected: Chunk-Alignment Logic and Resolution-Fallback Computation #124

@github-actions

Description

@github-actions

Analysis of recent commits on main

Assignee: @copilot

Summary

Two overlapping duplication patterns were found in fidnii/src/ClipPlanes.ts and fidnii/src/ResolutionSelector.ts:

  1. The core chunk-alignment computation (aligning a PixelRegion start/end to chunk boundaries) is implemented twice as two separate exported functions.
  2. The "region → aligned region → dimensions" computation block inside the for-loop bodies of selectResolution and select2DResolution is copy-pasted verbatim into both functions' fallback blocks (executed when every resolution exceeds maxPixels).

Pattern 1 — Duplicate Chunk-Alignment Logic

  • Severity: Medium
  • Occurrences: 2 identical implementations

Locations

File Function Lines
fidnii/src/ResolutionSelector.ts alignRegionToChunks 166–200
fidnii/src/ClipPlanes.ts alignToChunks 501–547

What is duplicated (~20 lines, identical algorithm)

Both functions:

  1. Compute chunkShape and volumeShape from the same helpers (getChunkShape, getVolumeShape).
  2. Align region.start down to the nearest chunk boundary with Math.floor(…/chunkSize) * chunkSize.
  3. Align region.end up to the nearest chunk boundary, clamped by Math.min(…, volumeShape[axis]).

The only behavioural difference is the return type:

  • alignRegionToChunks returns PixelRegion { start, end } (the aligned values).
  • alignToChunks returns ChunkAlignedRegion { start, end, chunkAlignedStart, chunkAlignedEnd, needsClipping } (original + aligned values + a flag).

Code Sample

// fidnii/src/ResolutionSelector.ts  alignRegionToChunks (lines 166-200)
const alignedStart: [number, number, number] = [
  Math.floor(region.start[0] / chunkShape[0]) * chunkShape[0],
  Math.floor(region.start[1] / chunkShape[1]) * chunkShape[1],
  Math.floor(region.start[2] / chunkShape[2]) * chunkShape[2],
]
const alignedEnd: [number, number, number] = [
  Math.min(Math.ceil(region.end[0] / chunkShape[0]) * chunkShape[0], volumeShape[0]),
  Math.min(Math.ceil(region.end[1] / chunkShape[1]) * chunkShape[1], volumeShape[1]),
  Math.min(Math.ceil(region.end[2] / chunkShape[2]) * chunkShape[2], volumeShape[2]),
]

// fidnii/src/ClipPlanes.ts  alignToChunks (lines 508-528) — byte-for-byte the same computation
const chunkAlignedStart: [number, number, number] = [
  Math.floor(region.start[0] / chunkShape[0]) * chunkShape[0],
  Math.floor(region.start[1] / chunkShape[1]) * chunkShape[1],
  Math.floor(region.start[2] / chunkShape[2]) * chunkShape[2],
]
const chunkAlignedEnd: [number, number, number] = [
  Math.min(Math.ceil(region.end[0] / chunkShape[0]) * chunkShape[0], volumeShape[0]),
  Math.min(Math.ceil(region.end[1] / chunkShape[1]) * chunkShape[1], volumeShape[1]),
  Math.min(Math.ceil(region.end[2] / chunkShape[2]) * chunkShape[2], volumeShape[2]),
]

Refactoring Recommendation

Consolidate into a single utility function (e.g. in fidnii/src/utils/ or ResolutionSelector.ts) that returns the aligned {start, end}. alignToChunks can then call it and derive needsClipping from the result:

// Proposed shared primitive
function alignBoundsToChunks(region: PixelRegion, ngffImage: NgffImage): PixelRegion {  }

// alignToChunks becomes a thin wrapper
export function alignToChunks(region, ngffImage): ChunkAlignedRegion {
  const { start: chunkAlignedStart, end: chunkAlignedEnd } =
    alignBoundsToChunks(region, ngffImage)
  const needsClipping = /* compare against region.start / region.end */
  return { start: region.start, end: region.end, chunkAlignedStart, chunkAlignedEnd, needsClipping }
}

Pattern 2 — Duplicated Region/Dimensions Block in Fallback Path

  • Severity: Low–Medium
  • Occurrences: 4 (once per each function × 2 functions)

Locations

File Function Loop body (lines) Fallback block (lines)
fidnii/src/ResolutionSelector.ts selectResolution 47–59 74–86
fidnii/src/ResolutionSelector.ts select2DResolution 288–302 310–324

What is duplicated (~12 lines each)

Both functions compute region → alignedRegion → dimensions inside their for loop, then copy the identical block verbatim in the after-loop fallback:

const region = clipPlanesToPixelRegion(clipPlanes, volumeBounds, image, viewportBounds)
const alignedRegion = alignRegionToChunks(region, image)
const dimensions: [number, number, number] = [
  alignedRegion.end[0] - alignedRegion.start[0],
  alignedRegion.end[1] - alignedRegion.start[1],
  alignedRegion.end[2] - alignedRegion.start[2],
]

Refactoring Recommendation

Extract a helper computeAlignedDimensions(image, …): { dimensions, … } or restructure the loop to always execute one final iteration for the lowest level without duplicating the computation block.


Impact Analysis

  • Maintainability: A bug fix or behaviour change in the chunk-alignment formula (e.g. for edge cases at volume boundaries) must be applied in two places; it is easy to miss one.
  • Bug Risk: alignToChunks (ClipPlanes) and alignRegionToChunks (ResolutionSelector) can silently diverge over time.
  • Code Bloat: ~50 lines of avoidable duplication across two source files.

Implementation Checklist

  • Review duplication findings
  • Consolidate alignToChunks / alignRegionToChunks into a shared primitive (Pattern 1)
  • Eliminate duplicated fallback blocks in selectResolution / select2DResolution (Pattern 2)
  • Update tests to cover the consolidated helpers
  • Verify no functionality is broken

Analysis Metadata

  • Analyzed Files: fidnii/src/ClipPlanes.ts, fidnii/src/ResolutionSelector.ts
  • Detection Method: Serena semantic code analysis (LSP symbol lookup + pattern search)
  • Analysis Date: 2026-04-08

Generated by Duplicate Code Detector

To install this agentic workflow, run

gh aw add github/gh-aw/.github/workflows/duplicate-code-detector.md@33cd6c7f1fee588654ef19def2e6a4174be66197

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions