adding validation in evaluator
Proposed Changes
Fixes https://github.com/ohcnetwork/roadmap/issues/137
Only PR's with test cases included and passing lint and test pipelines will be reviewed
@ohcnetwork/care-backend-maintainers @ohcnetwork/care-backend-admins
Summary by CodeRabbit
-
New Features
- Added operation-specific validation for evaluation rules, covering age (equality and range), gender (equality), and encounter tags.
- Provides clearer, user-friendly error messages for invalid inputs (e.g., non-numeric ages, invalid ranges, empty gender, malformed IDs).
- Extensible validation framework enables consistent checks across metrics.
-
Bug Fixes
- Prevents misconfiguration-related errors by blocking invalid rule values before evaluation.
- Improves stability and predictability when setting up evaluation criteria.
📝 Walkthrough
Walkthrough
Adds operation-specific validation to evaluation metrics via a class-level validators mapping in the base class. Subclasses wire appropriate validators for age, gender, and encounter tag metrics. New validator functions are introduced to enforce type/format constraints, and the base validation flow optionally invokes them.
Changes
| Cohort / File(s) | Change Summary |
|---|---|
Base validation hookcare/utils/evaluators/evaluation_metric/base.py |
Adds class-level validators = {}; validate_rule now optionally calls an operation-specific validator if present after operation verification. |
Metric-specific validator wiringcare/utils/evaluators/evaluation_metric/encounter_tag.py, care/utils/evaluators/evaluation_metric/patient_age.py, care/utils/evaluators/evaluation_metric/patient_gender.py |
Imports validator functions and defines validators dicts in each metric class mapping AllowedOperations.*.value to corresponding validator. No method signature changes. |
Validator implementationscare/utils/evaluators/evaluation_metric/validators.py |
Adds validate_patient_age_equality, validate_patient_age_in_range, validate_patient_gender_equality, validate_encounter_tag_has_tag with type/format checks (including UUID4). |
Sequence Diagram(s)
sequenceDiagram
autonumber
actor Caller
participant Metric as MetricSubclass
participant Base as EvaluationMetricBase
participant V as validators.py
Caller->>Metric: evaluate(rule)
Metric->>Base: validate_rule(rule)
Base->>Base: verify operation in AllowedOperations
alt validator mapped
Base->>V: call validator(value)
V-->>Base: ok or raise ValueError
else no validator
Note over Base: Skip operation-specific validation
end
Base-->>Metric: validation result
Metric-->>Caller: proceed or error
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
A guard at the gate, quiet and keen,
Validators whisper, “keep inputs clean.”
Age, gender, tags line up in queue—
UUIDs checked, and ranges too.
The base nods once, “you may proceed,”
Rules now behave, with far less need to plead.
Pre-merge checks and finishing touches
❌ Failed checks (2 warnings)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Description Check | ⚠️ Warning | The pull request description remains the untouched template with placeholder text and provides no actual details on the validation additions, related issue tracking, architecture notes, or completed merge checklist items, so it leaves reviewers guessing at the actual implementation. | Please fill in each section with a concise summary of the validation features added, link and explain the associated issue, remove or update the architecture changes section as needed, and mark off completed tests, documentation, and linting steps in the merge checklist. |
| Docstring Coverage | ⚠️ Warning | Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. | You can run @coderabbitai generate docstrings to improve docstring coverage. |
✅ Passed checks (1 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title Check | ✅ Passed | The pull request title “adding validation in evaluator” succinctly captures the primary change of introducing validation logic into the evaluator components, so it accurately reflects the core update. |
✨ Finishing touches
- [ ] 📝 Generate Docstrings
🧪 Generate unit tests
- [ ] Create PR with unit tests
- [ ] Post copyable unit tests in a comment
- [ ] Commit unit tests in branch
prafful/adding-validation-in-evaluator
[!TIP]
👮 Agentic pre-merge checks are now available in preview!
Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
- Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
- Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.
Please see the documentation for more information.
Example:
reviews: pre_merge_checks: custom_checks: - name: "Undocumented Breaking Changes" mode: "warning" instructions: | Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post.
Comment @coderabbitai help to get the list of available commands and usage tips.