Skip to content

Duplicate Code: Chunk-Alignment Logic in ClipPlanes.ts and ResolutionSelector.ts #126

@github-actions

Description

@github-actions

Summary

The chunk-boundary alignment algorithm is duplicated between alignToChunks (ClipPlanes.ts) and alignRegionToChunks (ResolutionSelector.ts). Both functions compute identical alignedStart and alignedEnd arrays using the same Math.floor/Math.ceil+Math.min pattern over getChunkShape and getVolumeShape results.

Analysis of recent commits to main (merge of PR #115).

Duplication Details

Pattern: Identical chunk-boundary clamping math

  • Severity: Medium
  • Occurrences: 2 functions, 18+ identical lines each
  • Locations:
    • fidnii/src/ClipPlanes.ts (lines 500–546) — alignToChunks
    • fidnii/src/ResolutionSelector.ts (lines 166–200) — alignRegionToChunks

Duplicated core block (appears verbatim in both functions):

const chunkShape = getChunkShape(ngffImage)
const volumeShape = getVolumeShape(ngffImage)

// Align start down to chunk boundary
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],
]

// Align end up to chunk boundary (but don't exceed volume size)
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]),
]

Differences between the two functions:

  • alignRegionToChunks returns { start: alignedStart, end: alignedEnd } (a PixelRegion with the clamped values)
  • alignToChunks returns a ChunkAlignedRegion that keeps the original region.start/end and adds chunkAlignedStart, chunkAlignedEnd, and a needsClipping boolean flag

Impact Analysis

  • Maintainability: Any fix to the clamping formula (e.g., off-by-one at volume edges, axis-order change) must be applied in two places and risks diverging.
  • Bug Risk: The functions are both exported from index.ts and called from OMEZarrNVImage.ts (4 call sites) and ResolutionSelector.ts (4 call sites). A silent divergence would be hard to detect.
  • Code Bloat: ~18 lines of duplicated math across two files.

Refactoring Recommendations

  1. Extract a shared helper (preferred):

    • Add a private/unexported function computeChunkAlignedBounds(region, ngffImage) in fidnii/src/ResolutionSelector.ts (where getChunkShape/getVolumeShape already live) that returns { alignedStart, alignedEnd }.
    • alignRegionToChunks calls it and returns { start: alignedStart, end: alignedEnd }.
    • alignToChunks (in ClipPlanes.ts) imports and calls the helper, then computes needsClipping from the result.
    • Estimated effort: ~30 min / low complexity.
  2. Alternative — implement alignToChunks in terms of alignRegionToChunks:

    • alignToChunks calls alignRegionToChunks to get the clamped region, then derives needsClipping by comparing with the original input.
    • This requires importing alignRegionToChunks into ClipPlanes.ts (currently it only imports helpers the other direction).

Implementation Checklist

  • Review duplication findings
  • Choose refactoring approach (shared helper vs. composition)
  • Extract common logic
  • Update alignToChunks and alignRegionToChunks to use shared helper
  • Verify all 8 call sites still pass type-checking (bunx tsc --noEmit from fidnii/)
  • Run bun run check to confirm linting/formatting pass
  • Run bun run test to confirm no regressions

Analysis Metadata

  • Analyzed Files: 14 source files in fidnii/src/
  • Detection Method: Serena semantic code analysis (symbol overview + body inspection)
  • Commit: 846ffc9 (HEAD → main)
  • Analysis Date: 2026-04-14

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