feat: add reusable CI validation utilities for development automation - #182
Conversation
Extracted from monster PR #171 as part of Phase 3A strategic decomposition. CI validation helpers providing: - Metric range validation (0-1 bounds checking) - Required keys validation with clear error messages - Attribute existence validation for objects - Declarative assertion helper for test conditions - Type-safe validation with specific exception types Reduces boilerplate in CI scripts and provides clearer failure semantics. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
Reviewer's GuideExtract reusable validation helpers for CI scripts into a dedicated module, providing type-safe utilities with clear error messages and declarative assertions. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
WalkthroughImplemented concrete validation logic in three functions within scripts/ci/validation_utils.py: validating metric ranges (0.0–1.0), ensuring required keys exist, and verifying required attributes on instances. Exceptions now include descriptive messages. Docstrings and formatting updated; no public API changes. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor CI as CI Script
participant VU as validation_utils
CI->>VU: validate_required_keys(obj, required)
alt missing key
VU-->>CI: raise KeyError("missing: ...")
else all present
VU-->>CI: return None
end
CI->>VU: validate_hasattrs(instance, attrs)
alt missing attribute
VU-->>CI: raise AttributeError("missing: ...")
else all present
VU-->>CI: return None
end
CI->>VU: validate_metric_ranges(metrics, required)
alt missing metric
VU-->>CI: raise ValueError("missing metric: ...")
else non-numeric or out of [0.0,1.0]
VU-->>CI: raise ValueError("invalid value: ...")
else all valid
VU-->>CI: return None
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello @uelkerd, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a dedicated module for reusable validation utilities, strategically extracted from a larger development effort. The primary goal is to enhance the robustness and maintainability of CI scripts by providing standardized functions for validating metric ranges, required dictionary keys, object attributes, and general conditions. These utilities are designed to reduce boilerplate code and offer more informative error feedback during automated checks, thereby streamlining the development automation process. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
Here's the code health analysis summary for commits Analysis Summary
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- Standardize the docstring style across all helpers (e.g. fully adopt NumPy or Google style) to keep parameter and raises sections consistent.
- Consider using Mapping[str, Any] instead of dict[str, Any] for the metric and key validators to allow broader input types (e.g. custom dict subclasses).
- The ensure() helper has a very generic name and could collide with other utilities—consider renaming it to something more descriptive (e.g.
assert_conditionorrequire) to avoid confusion.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Standardize the docstring style across all helpers (e.g. fully adopt NumPy or Google style) to keep parameter and raises sections consistent.
- Consider using Mapping[str, Any] instead of dict[str, Any] for the metric and key validators to allow broader input types (e.g. custom dict subclasses).
- The ensure() helper has a very generic name and could collide with other utilities—consider renaming it to something more descriptive (e.g. `assert_condition` or `require`) to avoid confusion.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Pull Request Overview
Adds reusable validation utilities for CI automation to reduce boilerplate and provide clearer error semantics. The module introduces validation functions for metric ranges, required keys, object attributes, and general assertions with type-specific exceptions.
- New validation utilities module with functions for metrics, keys, attributes, and conditions
- Improved docstring formatting using numpydoc-style "Raises" sections
- Type-safe design with specific exception types (ValueError, KeyError, AttributeError, AssertionError)
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| Raises | ||
| ------ |
There was a problem hiding this comment.
The docstring format should use 'Raises:' with a colon instead of 'Raises' with dashes for consistency with Python docstring conventions.
| Raises | ||
| ------ |
There was a problem hiding this comment.
The docstring format should use 'Raises:' with a colon instead of 'Raises' with dashes for consistency with Python docstring conventions.
| Raises | ||
| ------ |
There was a problem hiding this comment.
The docstring format should use 'Raises:' with a colon instead of 'Raises' with dashes for consistency with Python docstring conventions.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a set of reusable validation utility functions for CI scripts, with changes focused on improving code style through consistent docstring formatting and standardized function signatures. The stylistic changes are well-executed and improve readability. My feedback focuses on further enhancing the utility of these functions. I've suggested improvements for validate_required_keys and validate_hasattrs to provide more comprehensive error messages by reporting all missing items at once, rather than failing on the first one. This change would significantly improve the debugging experience, aligning with the PR's goal of providing 'clearer failure semantics'.
| def validate_required_keys( | ||
| obj: dict[str, Any], | ||
| keys: Iterable[str], | ||
| label: str = "object", | ||
| ) -> None: |
There was a problem hiding this comment.
The current implementation iterates through keys and fails on the first one that's missing. For a better developer experience, consider collecting all missing keys and reporting them in a single KeyError. This would allow a developer to fix all missing keys at once. You could achieve this efficiently using set operations to find the difference between the required keys and the keys present in the object.
| def validate_hasattrs( | ||
| instance: Any, | ||
| attrs: Iterable[str], | ||
| label: str = "object", | ||
| ) -> None: |
There was a problem hiding this comment.
Similar to validate_required_keys, this function fails on the first missing attribute. It would be more helpful to report all missing attributes in a single AttributeError. You could do this by iterating through all attributes, collecting the missing ones in a list, and then raising an exception with a consolidated error message if the list is not empty.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
scripts/ci/validation_utils.py (2)
18-28: Humanize missing-metric errors for consistencyRight now the out-of-range path prettifies the metric name, but the missing-metric path reports the raw key. Hoisting the prettified name keeps both error paths uniform and a bit friendlier.
for field in fields: + pretty = field.replace("_", " ").capitalize() if field not in metrics: - raise ValueError(f"Missing metric: {field}") + raise ValueError(f"{pretty} metric is missing") value = metrics[field] try: numeric = float(value) except (TypeError, ValueError) as exc: # non-numeric raise ValueError(f"Metric '{field}' is not numeric: {value}") from exc if not 0.0 <= numeric <= 1.0: - pretty = field.replace("_", " ").capitalize() raise ValueError(f"{pretty} should be between 0 and 1")
31-45: Broaden the key container toMappingWe only rely on
__contains__, so widening the annotation lets callers pass read-only mappings or other dict-like views without type ignore noise.-from typing import Any, Iterable +from typing import Any, Iterable, Mapping @@ -def validate_required_keys( - obj: dict[str, Any], +def validate_required_keys( + obj: Mapping[str, Any], keys: Iterable[str], label: str = "object", ) -> None:
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
scripts/ci/validation_utils.py(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Sourcery review
- GitHub Check: Analyze (python)
🔇 Additional comments (2)
scripts/ci/validation_utils.py (2)
2-5: Docstring context reads wellThe expanded module docstring concisely explains the intent behind these helpers and sets the right expectations for callers.
59-61: Attribute validation message is preciseGood job surfacing both the label and exact attribute name; the resulting
AttributeErrorreads clearly for CI logs.
Summary
Strategic extraction from monster PR #171 as part of Phase 3A focused decomposition.
Adds reusable validation helpers that reduce boilerplate and provide clearer failure semantics in CI scripts:
ensure()helper for clean test condition validationKey Features
Validation Types
Test Plan
🏰 Fortress-Protected Development: Single-purpose micro-PR (1 file, CI utility enhancement)
🤖 Generated with Claude Code
Summary by Sourcery
Extract and refine reusable CI validation utilities for checking metric ranges, required keys, object attributes, and declarative assertions with precise exception types and clear error messages
New Features:
Enhancements:
Documentation:
Summary by CodeRabbit