cleanup: dedup Alert/#109 + deprecate gradata.patterns shim/#110#114
cleanup: dedup Alert/#109 + deprecate gradata.patterns shim/#110#114
Conversation
…nitoring quality_monitoring.py carried an older, simpler copy of Alert + detect_failures + 4 detectors + format_alerts that parallel scoring/failure_detectors.py. No external callers — only self-references inside the module and its own __all__. scoring/failure_detectors is the canonical impl: - Alert has 4 fields (adds `evidence: dict`) vs. 3 here - Proper warning/critical severity escalation (this version only warned) - Field usage matches current MetricsWindow contract Scope of removal from quality_monitoring.py: - Alert dataclass - BLANDNESS_THRESHOLD constant - detect_being_ignored / detect_playing_safe / detect_overfitting / detect_regression_to_mean / detect_failures / format_alerts - Trimmed __all__ to just the anti-pattern surface - Updated module docstring to point at scoring/failure_detectors Kept (no duplicate, single impl): AntiPattern, AntiPatternDetector, Detection, DEFAULT_PATTERNS, DEFAULT_ANTI_PATTERNS, _compile_anti_pattern. Tests: 312 passed in test_adaptations.py + test_failure_detectors_coverage.py. Import smoke: AntiPatternDetector() still registers 13 default patterns. Closes #109. Co-Authored-By: Gradata <noreply@gradata.ai>
After 5b47d92 stripped detect_* functions from quality_monitoring.py, the SPEC compliance test asserted their presence on the wrong module. Pointing it at scoring/failure_detectors where they live canonically. Verified: test passes. Other failures in the full suite (test_agent_precontext_*, test_cli_export_*, test_runner_invokes_*) are pre-existing environmental flakes (live brain DB drift and Windows cp1252 encoding) unrelated to this branch. Co-Authored-By: Gradata <noreply@gradata.ai>
Council-decided (6-perspective, full mode). Two decisions: 1) gradata.patterns shim (#110): DeprecationWarning + remove at v0.8.0. - Runtime warning (not just docs) so users who pin versions and never read changelogs get the signal before the shim disappears. - Mirrors the deprecation cadence already set for gradata.integrations (also scheduled for v0.8.0 per #111). - Module-level _WARNED flag ensures one warning per process, not one per attribute access. - Council minority (Pragmatist) preferred 'keep indefinitely'; majority (Architect, Skeptic, Temporal, User Advocate) favored deprecation cycle. User Advocate's caveat — pair with release note — applies at release time. 2) #109 dedup guard: lock Alert identity in test_4_failure_detectors_exist. - Assert Alert.__module__ resolves to scoring.failure_detectors and that the evidence field is present — prevents accidental re-introduction of the stripped 3-field duplicate. - Addresses Architect's verification caveat: passing tests don't prove symbol identity; explicit __module__ assertion does. New test: test_patterns_shim_emits_deprecation_warning — guards against the DeprecationWarning being silently removed. Asserts both that the warning fires AND that it contains "v0.8.0" + "gradata.contrib.patterns" so the message stays actionable. Tests: 19/19 pattern + spec tests pass. Co-Authored-By: Gradata <noreply@gradata.ai>
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Gradata has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 Walkthrough
WalkthroughThis PR removes the duplicate Alert/alerting surface from Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/gradata/patterns/__init__.py`:
- Around line 15-17: Add "from __future__ import annotations" at the very top of
this module (before any other imports) so type annotations are postponed across
the core SDK; update src/gradata/patterns/__init__.py by inserting that future
import above the existing import warnings and keep the existing _WARNED variable
and imports unchanged.
In `@tests/test_bug_fixes.py`:
- Around line 387-409: The test test_patterns_shim_emits_deprecation_warning
collects warnings in caught but currently inspects deprecations[0] which may be
an unrelated DeprecationWarning; update the test to filter caught for
DeprecationWarning entries whose message contains the shim-specific text (e.g.
"gradata.contrib.patterns" or "v0.8.0") and then assert that this filtered list
is non-empty and that its first element's message contains the expected
substrings; reference the local variables caught and deprecations (or rename to
filtered_deprecations) and perform assertions on that filtered list instead of
deprecations[0].
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 782282ff-ef5c-4d52-b49c-539bdccb1e92
📒 Files selected for processing (4)
src/gradata/enhancements/quality_monitoring.pysrc/gradata/patterns/__init__.pytests/test_bug_fixes.pytests/test_spec_compliance.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
tests/**
⚙️ CodeRabbit configuration file
tests/**: Test files. Verify: no hardcoded paths, assertions check specific values not just truthiness,
parametrized tests preferred for boundary conditions, floating point comparisons use pytest.approx.
Files:
tests/test_spec_compliance.pytests/test_bug_fixes.py
src/gradata/**/*.py
⚙️ CodeRabbit configuration file
src/gradata/**/*.py: This is the core SDK. Check for: type safety (from future import annotations required), no print()
statements (use logging), all functions accepting BrainContext where DB access occurs, no hardcoded paths. Severity
scoring must clamp to [0,1]. Confidence values must be in [0.0, 1.0].
Files:
src/gradata/patterns/__init__.pysrc/gradata/enhancements/quality_monitoring.py
🧠 Learnings (2)
📓 Common learnings
Learnt from: Gradata
Repo: Gradata/gradata PR: 0
File: :0-0
Timestamp: 2026-04-17T17:18:07.417Z
Learning: In PR `#102` (gradata/gradata), Round 2 addressed: cli.py env-first brain resolution (GRADATA_BRAIN > --brain-dir > cwd), _tenant.py corrupt .tenant_id overwrite, _env_int default clamping to minimum, and _events.py tenant-scoped fallback SELECT for dedup. All ruff and 99 tests green after these fixes.
📚 Learning: 2026-04-17T17:18:07.417Z
Learnt from: Gradata
Repo: Gradata/gradata PR: 0
File: :0-0
Timestamp: 2026-04-17T17:18:07.417Z
Learning: In PR `#102` (gradata/gradata), Round 2 addressed: cli.py env-first brain resolution (GRADATA_BRAIN > --brain-dir > cwd), _tenant.py corrupt .tenant_id overwrite, _env_int default clamping to minimum, and _events.py tenant-scoped fallback SELECT for dedup. All ruff and 99 tests green after these fixes.
Applied to files:
tests/test_bug_fixes.py
🪛 Ruff (0.15.10)
src/gradata/patterns/__init__.py
[warning] 20-20: Missing return type annotation for private function __getattr__
(ANN202)
[warning] 21-21: Using the global statement to update _WARNED is discouraged
(PLW0603)
src/gradata/enhancements/quality_monitoring.py
[warning] 194-194: Boolean-typed positional argument in function definition
(FBT001)
[warning] 194-194: Boolean default positional argument in function definition
(FBT002)
tests/test_bug_fixes.py
[warning] 339-339: Boolean positional value in function call
(FBT003)
🔇 Additional comments (4)
src/gradata/patterns/__init__.py (1)
22-29: Deprecation shim behavior looks correct.One-time warning emission and two-step forwarding (
gradata.contrib.patternsthengradata.rules) are implemented cleanly and match the deprecation intent.Also applies to: 33-43
tests/test_bug_fixes.py (1)
354-378: Good coverage for deprecated shim forwarding.This import matrix meaningfully guards that
gradata.patternsstill forwards key symbols during deprecation.tests/test_spec_compliance.py (1)
202-213: Canonical Alert guard is well-implemented.The module-path assertion plus
evidencefield check is a strong regression guard against reintroducing the older 3-fieldAlert.src/gradata/enhancements/quality_monitoring.py (1)
1-7: Cleanup aligns with consolidation and keeps behavior intact.The docstring/API ownership clarification and local formatting refactors are clean and consistent with the Alert de-duplication objective.
Also applies to: 193-221
| import warnings | ||
|
|
||
| _WARNED = False |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify core SDK files that miss the required future-annotations import.
fd -e py . src/gradata | while read -r f; do
head -n 30 "$f" | rg -q '^from __future__ import annotations$' || echo "missing: $f"
doneRepository: Gradata/gradata
Length of output: 3074
🏁 Script executed:
head -n 50 src/gradata/patterns/__init__.pyRepository: Gradata/gradata
Length of output: 1324
Add from __future__ import annotations to this core SDK module.
Line 15 begins regular imports, but this file is missing the required future-annotations import used across src/gradata/**/*.py.
🔧 Proposed fix
+from __future__ import annotations
+
import warningsAs per coding guidelines: "src/gradata/**/*.py: This is the core SDK. Check for: type safety (from future import annotations required)".
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import warnings | |
| _WARNED = False | |
| from __future__ import annotations | |
| import warnings | |
| _WARNED = False |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/gradata/patterns/__init__.py` around lines 15 - 17, Add "from __future__
import annotations" at the very top of this module (before any other imports) so
type annotations are postponed across the core SDK; update
src/gradata/patterns/__init__.py by inserting that future import above the
existing import warnings and keep the existing _WARNED variable and imports
unchanged.
…t warning filter - Add `from __future__ import annotations` to patterns shim (SDK type-safety rule) - Move sys.modules.pop + reimport inside catch_warnings block so the shim DeprecationWarning is actually captured; filter caught list for shim-specific text before asserting, eliminating false-positive on unrelated DeprecationWarnings Co-Authored-By: Gradata <noreply@gradata.ai>
There was a problem hiding this comment.
Gradata has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
* docs: v0.6.1 changelog entry covering post-v0.6.0 work through PR #114 Covers: gradata.patterns deprecation (remove in v0.8.0), Alert dedup (#109), Meta-Harness A-D, multi-tenant SDK (#102), BM25 JIT ranking (#101), gradata seed/mine CLI, rule_verifier wiring, security hardening, and 67+66 ruff-violation fixes (#100, #103). Co-Authored-By: Gradata <noreply@gradata.ai> * docs(changelog): qualify env-centralization claim CodeRabbit flagged that GRADATA_RULE_VERIFIER is still read directly in rule_pipeline.py. Weaken the claim rather than block on the migration. Co-Authored-By: Gradata <noreply@gradata.ai> --------- Co-authored-by: Gradata <noreply@gradata.ai>
1 participant
Summary
Council-decided narrow cleanup PR, extracted from
autoresearch/consolidation(PR #112 will be closed in favor of this one). Three commits:ca45ec32— refactor(consolidation: unify duplicate Alert dataclass (quality_monitoring vs scoring/failure_detectors) #109): strip duplicate Alert/detect_failures from quality_monitoringRemoved the older 3-field
Alert+ 4 detector functions +detect_failures+format_alerts+BLANDNESS_THRESHOLDfromquality_monitoring.py. Canonical 4-field impl (withevidence: dict) lives inscoring/failure_detectors.py. Zero external callers of the stripped surface.bca932dc— test(consolidation: unify duplicate Alert dataclass (quality_monitoring vs scoring/failure_detectors) #109): update test_4_failure_detectors_exist to canonical modulePoints the SPEC compliance test at
scoring.failure_detectorsinstead ofquality_monitoring.6bafb89f— feat(consolidation: decide fate of hollow patterns/ shim vs canonical contrib/patterns/ #110): deprecate gradata.patterns shim + strengthen consolidation: unify duplicate Alert dataclass (quality_monitoring vs scoring/failure_detectors) #109 guardgradata.patternsnow emitsDeprecationWarningon first access; removal at v0.8.0 (mirrorsintegrations/cadence per consolidation: integrations/ orphan directory (no __init__.py) — merge into middleware/ or add contract #111).test_4_failure_detectors_existgainedAlert.__module__+evidencefield assertions to prevent re-introduction of the stripped duplicate.test_patterns_shim_emits_deprecation_warningguards the runtime warning.Council verdict (6-perspective, full mode)
Decision 1 (PR #112 strategy): 5× A, 1× D — cherry-pick safe commits to narrow PR off main. ✅ This PR.
Decision 2 (
patterns/shim): 4× B, 1× A, 1× D —DeprecationWarning+ remove at v0.8.0. ✅ Done.Addenda honored
__module__+evidencefield assertions.DeprecationWarningfires in__getattr__, not only in the docstring.Closes
Test plan
pytest tests/test_spec_compliance.py— 59 passedpytest tests/test_bug_fixes.py -k pattern— 1 passedpytest tests/test_bug_fixes.py::TestSDKExports::test_patterns_shim_emits_deprecation_warning— passes (new)pytest tests/test_adaptations.py tests/test_failure_detectors_coverage.py— 312 passedGenerated with Gradata