feat: add analytics utility functions - #32
Conversation
SummaryThis PR adds four general-purpose utility functions to a new What reviewers should knowThis is a test PR validating the SDK skills system for org review guidelines. The four utility functions are intentionally missing docstrings — this is by design to test whether review guidelines are properly triggered and delivered. Reviewers should focus on whether the skill-based guidelines are working as expected rather than on the utility functions themselves. Each function is straightforward: CSV parsing with delimiter support, windowed averaging with bounds checking, dictionary-based grouping, and value clamping.
|
SonarQube reviewer guide
|
![sonar-review-dev18[bot]](https://avatars.githubusercontent.com/in/3054993?s=60&v=4){kind=small}
| def parse_csv_line(line, delimiter=','): | ||
| parts = line.strip().split(delimiter) |
There was a problem hiding this comment.
Bug: Splitting on the raw delimiter without respecting quoted fields breaks on any CSV where a field value contains the delimiter — e.g. 'a,"b,c",d' splits into four parts instead of three, and the quote-stripping on line 3 doesn't recover because the split already happened.
Use csv.reader from the stdlib — it handles quoted fields, embedded delimiters, and escaped quotes correctly.
| def parse_csv_line(line, delimiter=','): | |
| parts = line.strip().split(delimiter) | |
| import csv | |
| import io | |
| def parse_csv_line(line, delimiter=','): | |
| reader = csv.reader(io.StringIO(line), delimiter=delimiter) | |
| return next(reader) |
- Mark as noise
|
|
||
|
|
||
| def clamp(value, minimum, maximum): | ||
| return max(minimum, min(maximum, value)) |
There was a problem hiding this comment.
Bug: When minimum > maximum the function silently returns minimum regardless of value — e.g. clamp(5, 10, 0) returns 10. An inverted range is almost certainly a caller mistake and should raise ValueError rather than silently producing a wrong result.
| return max(minimum, min(maximum, value)) | |
| def clamp(value, minimum, maximum): | |
| if minimum > maximum: | |
| raise ValueError("minimum must be <= maximum") | |
| return max(minimum, min(maximum, value)) |
- Mark as noise
| return [p.strip().strip('"') for p in parts] | ||
|
|
||
|
|
||
| def compute_moving_average(values, window): |
There was a problem hiding this comment.
Missing test coverage: compute_moving_average has the most complex business logic in this file — boundary validation, index arithmetic, and floating-point averaging. No tests cover:
- Window equal to
len(values)(should return a single average) - Window of 1 (should return the input unchanged)
- An empty
valueslist - Correct sliding behaviour (an off-by-one in
range(len(values) - window + 1)would be invisible without a test)
A parametrized unit test over these cases would catch regressions here.
- Mark as noise
1 participant
Test PR to validate that org review guidelines are delivered as SDK skills. Added
analytics.pywith 4 functions that deliberately have no docstrings.