Skip to content

feat: add data utility functions - #31

Open
CristianAmbrosini wants to merge 1 commit into
mainfrom
test/guidelines-validation-3
Open

feat: add data utility functions#31
CristianAmbrosini wants to merge 1 commit into
mainfrom
test/guidelines-validation-3

Conversation

@CristianAmbrosini

Copy link
Copy Markdown
Owner

Test PR to validate that org review guidelines are injected via ~/CLAUDE.md and picked up by the review agent.

@sonar-review-dev18

sonar-review-dev18 Bot commented Apr 23, 2026

Copy link
Copy Markdown

Summary

This PR adds a new data_utils.py module with four utility functions for common data manipulation tasks:

  • normalize(): Min-max normalization scaling values to [0, 1]
  • flatten(): Recursively flattens nested lists into a single level
  • chunk(): Splits a list into fixed-size sublists
  • merge_dicts(): Recursively merges two dictionaries with special handling for nested dict values

The PR is described as a test to validate that org review guidelines (from ~/CLAUDE.md) are properly injected and picked up by the review agent.

What reviewers should know

What to review:

  • The four utility function implementations in data_utils.py
  • Consider whether these functions are adequately tested (none currently included)
  • Check if there's a plan to integrate these into the codebase or if they're standalone examples

Things to note:

  • No unit tests are included with this addition
  • Functions lack docstrings—may want to confirm if that aligns with project standards
  • This is a new module that will need to be imported/used somewhere to be effective
  • The normalize() function will raise an exception if all values are identical (division by zero)

  • Generate Walkthrough
  • Generate Diagram

🗣️ Give feedback

@sonarqube-cloud-dev18

Copy link
Copy Markdown

SonarQube reviewer guide

Review in SonarQube

Quality Gate Failed Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

@sonar-review-dev18 sonar-review-dev18 Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are two real crash bugs in normalize() that need to be fixed before merge.

SonarQube Cloud status: Coverage on new code is 0% against an 80% threshold — see the reviewer guide above for details. The coverage gap is meaningful here: the uncovered edge cases in normalize() are exactly the paths that contain bugs.

🗣️ Give feedback

Comment thread data_utils.py
Comment on lines +1 to +4
def normalize(values):
mn = min(values)
mx = max(values)
return [(v - mn) / (mx - mn) for v in values]

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing test coverage: normalize() has two unguarded crash paths that no test currently exercises:

  • Empty listmin([]) raises ValueError. There is no guard before calling min/max.
  • All-identical values — when every element is the same, mx - mn == 0, causing ZeroDivisionError on the return line.

Both will blow up in production on valid-looking inputs. The uniform-values case is especially sneaky because a caller has no obvious reason to expect a crash.

A minimal fix for the zero-division case:

Suggested change
def normalize(values):
mn = min(values)
mx = max(values)
return [(v - mn) / (mx - mn) for v in values]
def normalize(values):
mn = min(values)
mx = max(values)
if mx == mn:
return [0.0] * len(values)
return [(v - mn) / (mx - mn) for v in values]
  • Mark as noise

Comment thread data_utils.py
return result


def chunk(lst, size):

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing test coverage: chunk(lst, 0) raises ValueError: range() arg 3 must not be zero with no helpful message. Since size comes from the caller, a zero or negative value is a plausible mistake. Add a guard and a clear error:

if size <= 0:
    raise ValueError(f"size must be a positive integer, got {size}")
  • Mark as noise

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant