Feature/Added morphological odd kernel size

[AbhaBarge]

Description

Implemented support for a configurable kernelSize parameter in the Morphological operator across both backend and frontend.

On the backend, the hardcoded 5×5 kernel was replaced with a dynamically generated kernel based on a user-provided kernelSize parameter. The parameter defaults to 5 to preserve backward compatibility and is validated to ensure it is a positive odd integer. Existing morph type handling and RGBA conversion logic were retained.

On the frontend, the Blockly morphological block was updated to include a numeric input field for kernel size alongside the morph type dropdown. The default value is set to 5, and frontend validation (odd-kernel-validator) guides users to enter positive odd integers only.

This enhancement allows users to control the scale of morphological operations while maintaining compatibility with existing pipelines and avoids any errors popping up due to user input.

Fixes #45

Type of Change

• Bug fix
• New feature
• Breaking change
• Documentation update

How Has This Been Tested?

Describe the tests you ran to verify your changes.

• Existing tests pass
• New tests added
• Manual testing

Screenshots (if applicable)

morph_1.mp4

Checklist

• My code follows the project's style guidelines
• I have performed a self-review
• I have added/updated documentation as needed
• My changes generate no new warnings
• Tests pass locally

[AbhaBarge]

@ivantha I've rebased this as well to the latest main. Branch is now mergeable. Please let me know if any changes are required. Thanks !

[ivantha]

◎ Argus Review

This PR adds configurable kernel size to the Morphological operator and introduces a new Histogram Equalization operator — both worthwhile features. However, there are two critical syntax/semantic errors in the frontend that will prevent compilation: a missing comma in categories.ts and a duplicate tooltip key in conversions.blocks.ts that corrupts the block definition with wrong text. A high-severity accidental deletion of the entire Sharpen block was also found, along with a Python type-validation bug that will reject valid float kernel sizes from JSON input. These blocking issues must be resolved before merging.

[ivantha]

[CRITICAL] Syntax Error: Missing Comma in Array Literal

The newly added histogramequalization entry is missing a trailing comma, producing an invalid TypeScript array literal. Two consecutive object literals without a separator is a compile-time syntax error that will prevent the frontend from building.

// Current (broken)
{ type: "imageconvertions_histogramequalization", label: "Histogram Equalization" }
{ type: "imageconvertions_bgrtohsv", label: "BGR to HSV" },

Suggestion:

Add a trailing comma after the new entry:
```ts
{ type: "imageconvertions_histogramequalization", label: "Histogram Equalization" },
{ type: "imageconvertions_bgrtohsv", label: "BGR to HSV" },

[ivantha]

[CRITICAL] Duplicate tooltip Key — Histogram Equalization Block Has Wrong Tooltip Text

The new histogramequalization block object is missing its closing },, so the old long colortobinary tooltip that was supposed to be deleted is still present in the file and appears as a second tooltip key inside the same block. In JavaScript/TypeScript, duplicate object keys are not a syntax error but the last value wins. The result is:

1. The histogramequalization block silently displays the colortobinary tooltip — factually wrong.
2. The intended short tooltip for histogram equalization is discarded without any compiler warning.

// What the new file actually contains:
{
  type: "imageconvertions_histogramequalization",
  ...
  tooltip: "Automatically improves image contrast..."   // ← intended (ignored)
  tooltip: "Convert colored (RGB) image to binary..."  // ← stale, wins!
},

Suggestion:

Remove the stale old `colortobinary` tooltip line that was not deleted. The correct block should be:
```ts
{
  type: "imageconvertions_histogramequalization",
  message0: "Enhance contrast using histogram equalization",
  previousStatement: null,
  nextStatement: null,
  style: "conversions_style",
  tooltip: "Automatically improves image contrast using histogram equalization",
},

[ivantha]

[HIGH] Accidental Deletion of the Entire Sharpen Block

The refactor of the morphological block appears to have accidentally deleted the entire filtering_sharpen block definition. The removed code included:

},{
  type: "filtering_sharpen",
  message0: "Apply sharpen with strength %1",
  args0: [
    { type: "field_number", name: "strength", value: 1.0, min: 0, max: 2, precision: 0.1 }
  ],
  previousStatement: null,
  nextStatement: null,
  style: "filtering_style",
  tooltip: "Applies image sharpening to enhance edges and details"
}

This is not mentioned in the PR description, checklist, or any commit. Removing this block silently breaks any existing user pipelines that use the sharpen operator and removes it from the toolbox entirely — a significant unintended breaking change.

Suggestion:

Restore the sharpen block by appending it after the morphological block:
```ts
  extensions: ["odd_kernel_validator"]
},
{
  type: "filtering_sharpen",
  message0: "Apply sharpen with strength %1",
  args0: [
    { type: "field_number", name: "strength", value: 1.0, min: 0, max: 2, precision: 0.1 }
  ],
  previousStatement: null,
  nextStatement: null,
  style: "filtering_style",
  tooltip: "Applies image sharpening to enhance edges and details"
}

[ivantha]

[HIGH] isinstance(int) Check Rejects Valid Float Kernel Sizes from JSON

The validation isinstance(kernel_size, int) is too strict. Python's JSON deserializer returns int for 5 and float for 5.0. Blockly's field_number code generator may emit a float representation, meaning the API request body could contain "kernelSize": 5.0. In Python 3, isinstance(5.0, int) returns False, so a ValueError is raised even though the value is semantically a valid odd integer.

>>> isinstance(5.0, int)
False
>>> int(5.0) % 2  # would be valid after coercion
1

This can cause silent pipeline failures in the wild where the frontend happens to serialize the kernel size with a decimal point.

Suggestion:

Coerce to `int` before the type/value check:
```python
try:
    kernel_size = int(self.params.get("kernelSize", 5))
except (TypeError, ValueError):
    kernel_size = 5

if kernel_size <= 0 or kernel_size % 2 == 0:
    raise ValueError("kernelSize must be a positive odd integer")

[ivantha]

[HIGH] RGBA / 4-Channel Images Silently Bypass Histogram Equalization

The compute method handles grayscale (2-channel) and BGR (3-channel) images but has no branch for BGRA/RGBA 4-channel images. If a 4-channel image is passed in, neither if condition matches and the method falls through to return image — silently returning the unprocessed original with no warning. This is a silent failure.

Notably, the Morphological operator in the same codebase explicitly guards against 4-channel images using NEEDS_RGB_CONVERSION, confirming that 4-channel images are a real and expected input in this system. Histogram Equalization should handle them consistently.

Suggestion:

Add a BGRA branch before the final `return image`:
```python
# BGRA image — strip alpha, equalize, then reattach alpha
if len(image.shape) == 3 and image.shape[2] == 4:
    bgr = cv2.cvtColor(image, cv2.COLOR_BGRA2BGR)
    bgr_eq = self.compute(bgr)  # recurse into the 3-channel path
    alpha = image[:, :, 3:4]
    return np.concatenate([bgr_eq, alpha], axis=2)

return image  # unsupported shape — return as-is

[ivantha]

[LOW] Missing Newline at End of File in Three Backend Files

Three files are flagged by git with \ No newline at end of file:

• imagelab-backend/app/operators/filtering/morphological.py
• imagelab-backend/app/operators/conversions/histogram_equalization.py
• imagelab-backend/app/operators/conversions/__init__.py

This violates POSIX standards, creates noisy diffs in future PRs, and may trigger CI linting checks (e.g., flake8 W391).

Suggestion:

Add a single trailing newline character (`\n`) to the end of each affected file. Configure your editor to do this automatically on save (most editors support `insertFinalNewline` in `.editorconfig`).

[ivantha]

[LOW] Detailed Tooltip Replaced with an Uninformative One-Liner

The original tooltip described each operation type in plain language:

'Open' removes small objects from the foreground, 'Close' fills small holes, 'Gradient' highlights edges, 'Tophat' extracts small bright details, 'Black hat' extracts small dark regions.

The replacement tooltip is:

Apply morphological transformation with adjustable kernel size

This is a notable usability regression. Non-expert users (the primary audience of a visual Blockly interface) rely on tooltips to understand which morph type to select. The PR should have retained the operation descriptions while appending kernel size guidance.

Suggestion:

Merge the original descriptions with new kernel size information:
```ts
tooltip: "Apply morphological transformation with a configurable kernel size. 'Open' removes small foreground objects, 'Close' fills small holes, 'Gradient' highlights edges, 'Top Hat' extracts bright details, 'Black Hat' extracts dark details. Use positive odd kernel sizes (3, 5, 7...); larger values affect bigger regions."

[ivantha]

[INFO] Good: Default of 5 Correctly Preserves Backward Compatibility

self.params.get("kernelSize", 5) uses the previously hard-coded 5×5 kernel as the default. Existing saved pipelines that do not include a kernelSize parameter will continue to behave identically to before. This is exactly the right approach when extending an existing operator — making the new parameter opt-in rather than required.

Suggestion:

No action needed. Consider adding a comment in the docstring noting that `kernelSize=5` is the backward-compatible default, e.g.: `kernelSize (int, optional): Size of the square structuring element. Must be a positive odd integer. Defaults to 5.`

[ivantha]

[INFO] Good: Correct Use of YCrCb Color Space for Color Histogram Equalization

Equalizing only the luminance (Y) channel in YCrCb color space while leaving the Cr/Cb chroma channels untouched is the technically correct and industry-standard approach. Naively equalizing each BGR channel independently would cause severe and visually unpleasant hue shifts. The inline comments (# Convert to YCrCb to preserve color fidelity, # Equalize only the luminance channel) are accurate and instructive.

Suggestion:

No action needed. As a minor enhancement, consider also supporting HSV as an alternative equalization space (equalizing the V channel), which some users may prefer for certain image types. This could be exposed as an optional parameter in a follow-up.

[ivantha]

[INFO] Good: Extension Registered Before Block Definitions — Correct Blockly Ordering

The odd_kernel_validator extension is registered via Blockly.Extensions.register(...) before Blockly.defineBlocksWithJsonArray(...) is called. This is the required ordering in Blockly's API — if a block definition references an extension name that hasn't been registered yet, Blockly throws a runtime error. The accompanying comment (// Registered odd kernel validator before defining blocks) makes the ordering intent explicit for future maintainers.

Suggestion:

No action needed. As the project grows, consider grouping all extension registrations into a dedicated `registerAllExtensions()` helper (similar to the existing `registerReadImageExtension()`) so this pattern stays consistent and discoverable across the codebase.

[ivantha]

◎ Argus Review

This PR introduces two features — a configurable kernelSize for the Morphological operator and a new Histogram Equalization operator — but contains several notable issues that should be addressed before merging. The most critical problems are duplicate imports in registry.py (two CropImage and two OtsuThreshold imports), a kernel size validation that will reject valid float values from JSON deserialization, and the new HistogramEqualization operator silently no-oping on RGBA/4-channel images rather than handling or rejecting them. There are also code quality concerns including dead code left from the validator refactor, an inadvertent truncation of an existing tooltip, missing trailing newlines, and a PR description that doesn't mention the Histogram Equalization feature at all despite it being a significant addition. The core feature logic is sound and backward-compatible, but the implementation needs cleanup before it can be considered production-ready.

Suggested Version Bump: minor

⚠️ Breaking Changes

• [INFO] Removal of self._kernel_size attribute may break subclasses: Previous behavior: The Morphological operator stored the kernel size as self._kernel_size (set in __init__) and used it in compute().

New behavior: compute() now reads kernelSize from self.params at call time; self._kernel_size (if still set in __init__) is no longer consumed.

Who is affected: Any subclass of Morphological that overrides compute() and references self._kernel_size directly, or any external code that reads the attribute.

Migration path: Update subclasses to read self.params.get('kernelSize', 5) instead of self._kernel_size.

Suggestion: If self._kernel_size is removed from __init__, document this in a CHANGELOG or migration guide. If it must be preserved for backward compatibility, retain the attribute but derive it from params:

def __init__(self, params: dict) -> None:
    super().__init__(params)
    # Kept for backward compatibility with subclasses
    self._kernel_size = self.params.get('kernelSize', 5)

PR Description Issues

• [MEDIUM] HistogramEqualization feature is entirely absent from PR description: The PR title and description focus exclusively on the morphological kernelSize parameter. However, the diff introduces a complete new HistogramEqualization operator (new Python class, registry entry, Blockly block, and category registration). This is a distinct feature that is not mentioned anywhere in the description, checklist, or testing section. A reviewer has no context on why this feature was added, how it was tested, or whether it's related to issue #45.

  Suggestion: Update the PR description to include the HistogramEqualization feature:

## Description
This PR makes two additions:

1. **Configurable kernel size for Morphological operator**: The hardcoded 5×5 kernel was replaced with a user-configurable `kernelSize` parameter...

2. **Histogram Equalization operator (new)**: Added a new operator that enhances image contrast automatically for both grayscale and color images using OpenCV's `equalizeHist` function applied to the luminance channel (YCrCb color space).

• [LOW] Testing section lacks specifics for new functionality: The PR checklist marks all test-related items as checked ([x] Existing tests pass, [x] New tests added, [x] Manual testing), but the description reads: "Describe the tests you ran to verify your changes." — which is the unfilled template placeholder text. No actual test descriptions are provided. Additionally, the diff does not include any new test files, which contradicts the [x] New tests added claim.

  Suggestion: Replace the placeholder with concrete test descriptions:

## How Has This Been Tested?
- **Unit tests**: Added `test_morphological_kernel_size.py` covering valid kernel sizes (1, 3, 5), invalid sizes (0, -1, 4), and default behavior (no kernelSize param).
- **Integration tests**: Verified the Blockly block serializes `kernelSize` correctly and the backend applies the correct kernel dimensions.
- **Manual testing**: Tested erosion/dilation with kernel sizes 3, 5, 9 on a sample image and confirmed expected morphological effects.

• [INFO] PR mixes two unrelated features in a single change: The PR introduces: (1) a configurable kernelSize for the Morphological operator, and (2) an entirely new Histogram Equalization operator. These are unrelated features that likely address different requirements. Combining them makes the PR harder to review, harder to bisect if a regression is introduced, and harder to revert cleanly.

  Suggestion: Consider splitting this into two PRs in the future:

• feature/morphological-configurable-kernel-size (fixes #45)
• feature/histogram-equalization-operator (separate issue/feature request)

If the changes must ship together, ensure the PR description and title clearly reflect both additions.

[ivantha]

[HIGH] Duplicate import of CropImage

The CropImage class is imported twice from the same module:

from app.operators.geometric.crop_image import CropImage  # line ~37 (original)
from app.operators.geometric.crop_image import CropImage  # line ~38 (duplicate added)

While Python silently ignores duplicate imports, this is indicative of a merge/copy-paste error and pollutes the module namespace. It can confuse static analysis tools, linters, and developers reading the file.

Suggestion:

Remove the duplicate import line:
```python
# Keep only one:
from app.operators.geometric.crop_image import CropImage

[ivantha]

[HIGH] Duplicate import of OtsuThreshold

The OtsuThreshold class is imported twice from the same module:

from app.operators.thresholding.otsu_threshold import OtsuThreshold  # original
from app.operators.thresholding.otsu_threshold import OtsuThreshold  # duplicate added in this PR

Same problem as the CropImage duplicate — likely a copy-paste error during a merge or rebase. These duplicate imports should be caught by a linter (e.g., flake8 --select=F811 or pylint).

Suggestion:

Remove the duplicate import. Consider adding a pre-commit hook or CI linting step with `flake8` or `isort` to prevent this in future.

[ivantha]

[HIGH] Validation rejects valid float kernel sizes from JSON deserialization

The validation uses isinstance(kernel_size, int), but JSON does not distinguish between integers and floats. Depending on how parameters are serialized by the Blockly frontend and deserialized in the backend, 5 in JSON may be parsed as a Python float (5.0) in some frameworks or transport layers. This would cause a ValueError even for a perfectly valid kernel size of 5:

if not isinstance(kernel_size, int) or kernel_size <= 0 or kernel_size % 2 == 0:
    raise ValueError("kernelSize must be a positive odd integer")

For example: isinstance(5.0, int) → False → raises ValueError unexpectedly.

Suggestion:

Coerce and validate more robustly:
```python
try:
    kernel_size = int(kernel_size)
except (TypeError, ValueError):
    raise ValueError("kernelSize must be a positive odd integer")
if kernel_size <= 0 or kernel_size % 2 == 0:
    raise ValueError("kernelSize must be a positive odd integer")

[ivantha]

[MEDIUM] RGBA (4-channel) images silently pass through without equalization

The compute method handles grayscale (2D) and BGR (3-channel) images, but silently returns the original image unchanged for RGBA (4-channel) images:

if len(image.shape) == 2:
    return cv2.equalizeHist(image)
if len(image.shape) == 3 and image.shape[2] == 3:
    # ... handles BGR
    return cv2.cvtColor(ycrcb, cv2.COLOR_YCrCb2BGR)
return image  # RGBA falls here — silently unprocessed!

Many images in the pipeline may carry an alpha channel (BGRA/RGBA), so this silent no-op is likely a hidden bug. Users will get no contrast enhancement and no error message.

Suggestion:

Handle the RGBA case explicitly:
```python
# RGBA image: equalize on luminance, preserve alpha
if len(image.shape) == 3 and image.shape[2] == 4:
    bgr = image[:, :, :3]
    alpha = image[:, :, 3]
    ycrcb = cv2.cvtColor(bgr, cv2.COLOR_BGR2YCrCb)
    ycrcb[:, :, 0] = cv2.equalizeHist(ycrcb[:, :, 0])
    bgr_eq = cv2.cvtColor(ycrcb, cv2.COLOR_YCrCb2BGR)
    return np.dstack([bgr_eq, alpha])

# Unsupported image format
raise ValueError(f"Unsupported image shape: {image.shape}")

[ivantha]

[MEDIUM] registerOddKernelValidator function is now dead code

The original registerOddKernelValidator() function is still defined in the file but its call has been replaced with inlined registration logic. This leaves the function as unreachable dead code:

function registerOddKernelValidator() {
  // This body still exists but is NEVER called
  Blockly.Extensions.register("odd_kernel_validator", ...);
}

export function registerAllBlocks() {
  registerReadImageExtension();
  // registerOddKernelValidator(); // removed
  Blockly.Extensions.register("odd_kernel_validator", ...); // inlined

This could lead to confusion for future maintainers who see a function defined but never called, and if the function is accidentally invoked elsewhere it would throw a Blockly error (duplicate extension registration).

Suggestion:

Remove the `registerOddKernelValidator` function definition entirely, keeping only the inlined registration within `registerAllBlocks`. Alternatively, revert the inlining and keep the named function — the original structure was cleaner.

[ivantha]

[LOW] Missing newline at end of file

The diff indicates \ No newline at end of file for morphological.py. POSIX standards require text files to end with a newline. This can cause issues with certain tools, git diffs, and linters.

Suggestion:

Add a trailing newline after the last line of the file. Many editors (VSCode, PyCharm) can be configured to do this automatically on save.

[ivantha]

[LOW] Missing newline at end of file

Similar to the morphological.py issue, histogram_equalization.py is also missing a trailing newline. This is flagged by flake8 (W292) and violates POSIX conventions.

Suggestion:

Add a trailing newline at the end of the file.

[ivantha]

[LOW] compute() method lacks a docstring

The class-level docstring is good, but the compute method itself has no docstring. This is inconsistent with good Python documentation practice and may differ from how other operators in the codebase are documented. Parameters, return values, and edge-case behaviors (especially the silent no-op for unsupported shapes) should be documented.

Suggestion:

```python
def compute(self, image: np.ndarray) -> np.ndarray:
    """
    Apply histogram equalization to enhance image contrast.

    Args:
        image: Input image as a NumPy array (grayscale, BGR, or BGRA).

    Returns:
        Contrast-enhanced image in the same format as the input.
        Returns the original image unchanged if format is not supported.
    """

[ivantha]

[LOW] No tests for edge cases in kernel size validation

The PR adds server-side validation for kernelSize, but there's no evidence in the diff of test cases covering:

• kernelSize = 0 (zero — invalid)
• kernelSize = -3 (negative — invalid)
• kernelSize = 4 (even — invalid)
• kernelSize = 5.0 (float — currently rejected by isinstance check, possibly unintentionally)
• kernelSize = 1 (valid minimal case)
• kernelSize absent from params (default=5 path)

The PR checklist claims "New tests added" but the diff doesn't show any test files.

Suggestion:

Add parametrized tests:
```python
@pytest.mark.parametrize("kernel_size,should_raise", [
    (5, False), (1, False), (3, False),
    (0, True), (-1, True), (4, True), (6, True),
])
def test_morphological_kernel_size_validation(kernel_size, should_raise):
    op = Morphological({"type": "TOPHAT", "kernelSize": kernel_size})
    if should_raise:
        with pytest.raises(ValueError):
            op.compute(sample_image)
    else:
        result = op.compute(sample_image)
        assert result is not None

[ivantha]

[INFO] Even-number correction increments rather than rounding to nearest odd

The validator corrects even kernel sizes by incrementing (value += 1), so entering 4 yields 5 and entering 6 yields 7. An alternative UX approach would be to round to the nearest odd integer (e.g., 4 → 3 or 4 → 5). The current behavior is deterministic but may surprise users who expect rounding-down behavior. The comment // Force odd number does not explain the direction of correction.

Suggestion:

At a minimum, improve the comment to clarify the behavior:
```typescript
// Round up to next odd number if even (e.g., 4 → 5, 6 → 7)
if (value % 2 === 0) {
  value += 1;
}

Alternatively, consider rounding to nearest odd:

if (value % 2 === 0) {
  value = value > 1 ? value - 1 : value + 1;
}

[github-actions]

PR Review

Rebase

Merge commits detected — please use rebase instead of merge:

5a273ad Merge branch 'main' into feature/morphological_odd_kernel_size

Squash

Your PR has 3 commits. Please squash into a single commit.

How to fix

git fetch origin
git rebase -i origin/main   # mark all but first commit as "squash"
git push --force-with-lease

---

This comment updates automatically on each push.

[ivantha]

Thanks for the PR! It currently has merge conflicts with main. Could you rebase onto the latest main and push the updated branch? Happy to merge once it's auto-rebasable.