feat: implement unified deprecation management system#1192
Merged
Conversation
- Create centralized @deprecated decorator with version tracking - Add warn_deprecated_param() for consistent parameter warnings - Add MIGRATION.md with comprehensive deprecation guide - Update 7 key files to use unified deprecation system - Add version-based removal timeline (deprecated in 1.0.0, removed in 2.0.0) - Ensure backward compatibility with zero breaking changes - Add expiration detection for future CI integration Fixes #1135 🤖 Generated with [Claude Code](https://claude.ai/code) Co-authored-by: Mervin Praison <MervinPraison@users.noreply.github.com>
Owner
|
@coderabbitai review |
Owner
|
/review |
Contributor
✅ Actions performedReview triggered.
|
Code Review by Qodo
1. check_expired_deprecations() is placeholder
|
Contributor
📝 WalkthroughWalkthroughThis PR implements a centralized deprecation strategy by introducing a new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
Contributor
Author
|
Claude encountered an error —— View job Code Review for Unified Deprecation Management System
|
Contributor
Author
|
Claude encountered an error —— View job Analyzing PR and Repository Guidelines
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+188
to
+212
| def check_expired_deprecations(current_version: Optional[str] = None) -> list[str]: | ||
| """ | ||
| Check for expired deprecations in the codebase. | ||
|
|
||
| This function can be used in CI to detect deprecations that should be removed. | ||
|
|
||
| Args: | ||
| current_version: Override for current version (defaults to DeprecationConfig.CURRENT_VERSION) | ||
|
|
||
| Returns: | ||
| List of expired deprecation messages | ||
| """ | ||
| # This is a placeholder for CI integration | ||
| # In practice, this would scan the codebase for @deprecated decorators | ||
| # and check their removal versions against the current version | ||
| if current_version is None: | ||
| current_version = DeprecationConfig.CURRENT_VERSION | ||
|
|
||
| expired = [] | ||
|
|
||
| # TODO: Add scanning logic here for CI integration | ||
| # This would use AST parsing to find all @deprecated decorators | ||
| # and check if their removal versions have passed | ||
|
|
||
| return expired |
There was a problem hiding this comment.
1. check_expired_deprecations() is placeholder 📎 Requirement gap ⛯ Reliability
check_expired_deprecations() is explicitly a placeholder and currently cannot detect or fail CI on deprecations past their removal version. This allows overdue deprecations to merge/release without automated enforcement.
Agent Prompt
## Issue description
`check_expired_deprecations()` is a placeholder and does not detect or fail builds for expired deprecations.
## Issue Context
Compliance requires CI enforcement to prevent shipping APIs past their planned `removal` version.
## Fix Focus Areas
- src/praisonai-agents/praisonaiagents/utils/deprecation.py[188-212]
- .github/workflows/test-core.yml[36-56]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+15
to
+25
| class DeprecationConfig: | ||
| """Configuration for deprecation behavior and version management.""" | ||
|
|
||
| # Current version for deprecation tracking | ||
| CURRENT_VERSION = "1.0.0" | ||
|
|
||
| # Show stack traces for deprecation warnings (useful for debugging) | ||
| SHOW_STACKLEVEL = True | ||
|
|
||
| # Warn about expired deprecations that should be removed | ||
| WARN_EXPIRED = True |
There was a problem hiding this comment.
2. Hardcoded current_version 🐞 Bug ✓ Correctness
DeprecationConfig.CURRENT_VERSION is hardcoded to "1.0.0", so expiration checks and "EXPIRED DEPRECATION" warnings will be wrong for the actually shipped package version. This breaks the PR’s version-tracking/CI goal because deprecations will not expire based on the real installed version.
Agent Prompt
### Issue description
`DeprecationConfig.CURRENT_VERSION` is hardcoded to `"1.0.0"`, but the package version is `1.5.87`. This makes deprecation expiry detection incorrect.
### Issue Context
The PR introduces version-aware deprecations and a CI hook (`check_expired_deprecations`). To be meaningful, the “current version” must reflect the installed distribution version (or be configurable).
### Fix Focus Areas
- src/praisonai-agents/praisonaiagents/utils/deprecation.py[15-40]
- src/praisonai-agents/pyproject.toml[5-12]
### Suggested fix
- Replace the constant with a runtime-derived value using `importlib.metadata.version("praisonaiagents")` (with a safe fallback like `"0.0.0"` on `PackageNotFoundError`).
- Optionally allow overriding via env var (e.g. `PRAISONAIAGENTS_DEPRECATION_CURRENT_VERSION`) for CI/testing.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Contributor
Author
|
@copilot Do a thorough review of this PR. Read ALL existing reviewer comments above first. Review areas:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
src/praisonai-agents/praisonaiagents/utils/deprecation.py (4)
28-32: Version parsing may fail on pre-release versions.
_parse_versionwill raiseValueErrorif a version contains non-numeric segments like"1.0.0-alpha"or"1.0.0rc1". If the project uses such versions, consider adding error handling.📝 Defensive parsing
def _parse_version(version_str: str) -> tuple[int, ...]: """Parse version string into comparable tuple.""" if not version_str: return (0,) - return tuple(int(x) for x in version_str.split('.')) + # Strip pre-release/build metadata (e.g., "1.0.0-alpha" -> "1.0.0") + base_version = version_str.split('-')[0].split('+')[0] + try: + return tuple(int(x) for x in base_version.split('.')) + except ValueError: + return (0,) # Fallback for unparseable versions🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/praisonai-agents/praisonaiagents/utils/deprecation.py` around lines 28 - 32, The _parse_version function will raise ValueError on pre-release or non-numeric segments (e.g., "1.0.0-alpha" or "1.0.0rc1"); update _parse_version to defensively extract only the numeric dot-separated components (or fallback gracefully) by stripping any prerelease/build suffixes or by using a regex to capture the leading numeric groups, converting those to ints and returning a tuple, and if parsing fails return a safe default like (0,) or the numeric prefix; ensure this change targets the _parse_version function so comparisons remain robust for versions with non-numeric segments.
188-212: Placeholder implementation is acceptable, but consider documenting the gap.The
check_expired_deprecationsfunction is exported as public API but returns an empty list. While the TODO explains future plans, callers might be surprised by the no-op behavior. Consider either:
- Raising
NotImplementedErrorto make it explicit- Adding a note in the docstring that this is not yet implemented
📝 Option: Make placeholder behavior explicit
Returns: List of expired deprecation messages + + Note: + This function is a placeholder for future CI integration. + Currently returns an empty list. See TODO for implementation details. """🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/praisonai-agents/praisonaiagents/utils/deprecation.py` around lines 188 - 212, The public function check_expired_deprecations currently returns an empty list as a placeholder which can surprise callers; either make the no-op explicit by raising NotImplementedError from check_expired_deprecations (so callers get a clear failure) or update its docstring to state it is intentionally unimplemented and will always return an empty list until CI-scanning logic is added; locate the function by name check_expired_deprecations in deprecation.py and implement one of those two fixes and ensure the docstring mentions the chosen behavior and the expected future AST-scanning enhancement.
13-13:warn_deprecated_paramis missing from__all__.The function
warn_deprecated_paramis used across multiple files (agent.py, task.py, paths.py, etc.) and is re-exported inutils/__init__.py, but it's not included in this module's__all__. This inconsistency could cause issues with static analysis tools and IDE autocompletion.📝 Suggested fix
-__all__ = ['deprecated', 'DeprecationConfig', 'check_expired_deprecations'] +__all__ = ['check_expired_deprecations', 'deprecated', 'DeprecationConfig', 'warn_deprecated_param']This also addresses the static analysis hint about sorting
__all__alphabetically.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/praisonai-agents/praisonaiagents/utils/deprecation.py` at line 13, Add the missing symbol warn_deprecated_param to the module export list by including it in the __all__ definition alongside deprecated, DeprecationConfig, and check_expired_deprecations (i.e., __all__ = ['deprecated', 'DeprecationConfig', 'check_expired_deprecations', 'warn_deprecated_param']); also reorder the entries alphabetically to satisfy static analysis/IDE expectations. Ensure the exact function name warn_deprecated_param matches the symbol exported elsewhere (and update any tests/imports if necessary).
126-185: Docstring example could clarifystackleveladjustment.The docstring shows
stacklevelusage, but doesn't mention that callers typically need to increase it when calling from within another function (e.g.,__init__). Since the AI summary indicates call sites adjust this value, consider adding a note.📝 Enhanced docstring
Args: param_name: Name of the deprecated parameter since: Version when parameter was deprecated removal: Version when parameter will be removed alternative: Suggested replacement details: Additional migration guidance - stacklevel: Stack level for warning (default: 2) + stacklevel: Stack level for warning (default: 2). Increase by 1 for + each additional call frame between this function and user code + (e.g., use 3 when calling from an __init__ method).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/praisonai-agents/praisonaiagents/utils/deprecation.py` around lines 126 - 185, Update the docstring for warn_deprecated_param to explicitly document that stacklevel controls where the warning appears in user stack traces and that callers who wrap this helper (e.g., from __init__ or other helper functions) should increase stacklevel to point at the original caller (showing a short example using stacklevel=3 when calling from a wrapper). Mention that the default is 2 and that adjusting it preserves correct file/line attribution for end users.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@MIGRATION.md`:
- Around line 122-131: Replace the invalid example `result = process='workflow'`
with the actual deprecated call form so the "Old Way" is valid Python; for
example change it to a function-style invocation like `result =
process('workflow')` (or to the real deprecated API if different), keeping the
rest of the snippet showing `from praisonaiagents import process` and
contrasting it with the new `Workflow`/`workflow.start()` usage.
In `@src/praisonai-agents/praisonaiagents/utils/deprecation.py`:
- Around line 15-26: DeprecationConfig currently hardcodes CURRENT_VERSION =
"1.0.0" which drifts from the real package version; change it to read the
installed package version at module import (use
importlib.metadata.version("praisonaiagents") or importlib.metadata.version with
a try/except) and assign that to DeprecationConfig.CURRENT_VERSION (e.g., set a
module-level _CURRENT_VERSION from importlib.metadata.version("praisonaiagents")
with a safe fallback like "0.0.0" or the existing default on exception, then
have class DeprecationConfig reference that variable) so expired deprecation
detection uses the actual package version.
---
Nitpick comments:
In `@src/praisonai-agents/praisonaiagents/utils/deprecation.py`:
- Around line 28-32: The _parse_version function will raise ValueError on
pre-release or non-numeric segments (e.g., "1.0.0-alpha" or "1.0.0rc1"); update
_parse_version to defensively extract only the numeric dot-separated components
(or fallback gracefully) by stripping any prerelease/build suffixes or by using
a regex to capture the leading numeric groups, converting those to ints and
returning a tuple, and if parsing fails return a safe default like (0,) or the
numeric prefix; ensure this change targets the _parse_version function so
comparisons remain robust for versions with non-numeric segments.
- Around line 188-212: The public function check_expired_deprecations currently
returns an empty list as a placeholder which can surprise callers; either make
the no-op explicit by raising NotImplementedError from
check_expired_deprecations (so callers get a clear failure) or update its
docstring to state it is intentionally unimplemented and will always return an
empty list until CI-scanning logic is added; locate the function by name
check_expired_deprecations in deprecation.py and implement one of those two
fixes and ensure the docstring mentions the chosen behavior and the expected
future AST-scanning enhancement.
- Line 13: Add the missing symbol warn_deprecated_param to the module export
list by including it in the __all__ definition alongside deprecated,
DeprecationConfig, and check_expired_deprecations (i.e., __all__ =
['deprecated', 'DeprecationConfig', 'check_expired_deprecations',
'warn_deprecated_param']); also reorder the entries alphabetically to satisfy
static analysis/IDE expectations. Ensure the exact function name
warn_deprecated_param matches the symbol exported elsewhere (and update any
tests/imports if necessary).
- Around line 126-185: Update the docstring for warn_deprecated_param to
explicitly document that stacklevel controls where the warning appears in user
stack traces and that callers who wrap this helper (e.g., from __init__ or other
helper functions) should increase stacklevel to point at the original caller
(showing a short example using stacklevel=3 when calling from a wrapper).
Mention that the default is 2 and that adjusting it preserves correct file/line
attribution for end users.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2e46d08c-576b-4b6b-a406-3816867fb256
📒 Files selected for processing (10)
MIGRATION.mdsrc/praisonai-agents/praisonaiagents/agent/agent.pysrc/praisonai-agents/praisonaiagents/agent/autonomy.pysrc/praisonai-agents/praisonaiagents/paths.pysrc/praisonai-agents/praisonaiagents/process/process.pysrc/praisonai-agents/praisonaiagents/task/task.pysrc/praisonai-agents/praisonaiagents/utils/__init__.pysrc/praisonai-agents/praisonaiagents/utils/deprecation.pysrc/praisonai/praisonai/bots/_approval.pysrc/praisonai/praisonai/llm/__init__.py
Comment on lines
+122
to
+131
| ```python | ||
| # ❌ Old Way | ||
| from praisonaiagents import process | ||
| result = process='workflow' | ||
|
|
||
| # ✅ New Way | ||
| from praisonaiagents import Workflow | ||
| workflow = Workflow(steps=[...]) | ||
| result = workflow.start() | ||
| ``` |
Contributor
There was a problem hiding this comment.
Syntax error in "Old Way" code example.
Line 125 contains invalid Python syntax: result = process='workflow'. This appears to be a typo or incomplete example that would confuse users trying to understand the deprecated pattern.
📝 Suggested fix
Please correct this example to show the actual deprecated usage pattern. Based on the context, it might be something like:
# ❌ Old Way
from praisonaiagents import process
-result = process='workflow'
+result = process.workflow(steps=[...])Or if this was meant to show a different pattern, please clarify the actual deprecated API.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@MIGRATION.md` around lines 122 - 131, Replace the invalid example `result =
process='workflow'` with the actual deprecated call form so the "Old Way" is
valid Python; for example change it to a function-style invocation like `result
= process('workflow')` (or to the real deprecated API if different), keeping the
rest of the snippet showing `from praisonaiagents import process` and
contrasting it with the new `Workflow`/`workflow.start()` usage.
Comment on lines
+15
to
+26
| class DeprecationConfig: | ||
| """Configuration for deprecation behavior and version management.""" | ||
|
|
||
| # Current version for deprecation tracking | ||
| CURRENT_VERSION = "1.0.0" | ||
|
|
||
| # Show stack traces for deprecation warnings (useful for debugging) | ||
| SHOW_STACKLEVEL = True | ||
|
|
||
| # Warn about expired deprecations that should be removed | ||
| WARN_EXPIRED = True | ||
|
|
Contributor
There was a problem hiding this comment.
CURRENT_VERSION is hardcoded and drifts from actual package version.
CURRENT_VERSION is set to "1.0.0" but the actual package version in pyproject.toml is "1.5.87". This drift undermines the expired deprecation detection intended for CI integration. For example, a deprecation with removal="1.3.0" would not be flagged as expired even though the actual version has passed it.
Consider dynamically loading the version from pyproject.toml or using importlib.metadata:
🔧 Suggested approach
+import importlib.metadata
+
class DeprecationConfig:
"""Configuration for deprecation behavior and version management."""
- # Current version for deprecation tracking
- CURRENT_VERSION = "1.0.0"
+ # Current version for deprecation tracking (dynamically loaded)
+ `@staticmethod`
+ def _get_version() -> str:
+ try:
+ return importlib.metadata.version("praisonaiagents")
+ except importlib.metadata.PackageNotFoundError:
+ return "0.0.0" # Fallback for development
+
+ CURRENT_VERSION = property(lambda self: DeprecationConfig._get_version())Or simpler, at module load time:
try:
import importlib.metadata
_CURRENT_VERSION = importlib.metadata.version("praisonaiagents")
except Exception:
_CURRENT_VERSION = "1.0.0"
class DeprecationConfig:
CURRENT_VERSION = _CURRENT_VERSION🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/praisonai-agents/praisonaiagents/utils/deprecation.py` around lines 15 -
26, DeprecationConfig currently hardcodes CURRENT_VERSION = "1.0.0" which drifts
from the real package version; change it to read the installed package version
at module import (use importlib.metadata.version("praisonaiagents") or
importlib.metadata.version with a try/except) and assign that to
DeprecationConfig.CURRENT_VERSION (e.g., set a module-level _CURRENT_VERSION
from importlib.metadata.version("praisonaiagents") with a safe fallback like
"0.0.0" or the existing default on exception, then have class DeprecationConfig
reference that variable) so expired deprecation detection uses the actual
package version.
Owner
|
@copilot Review this PR thoroughly. Check code quality, security, performance, backward compatibility, and naming conventions. |
Contributor
Author
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
Owner
|
@claude Final architectural review. Check: 1) Protocol-driven (no heavy impl in core) 2) Lazy imports 3) Thread/async safety 4) Backward compat 5) No scope creep beyond the issue. Approve or request changes. |
Contributor
Author
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements a centralized deprecation management strategy to address scattered deprecation warnings across 14 files with no consistent versioning or migration guidance.
What This PR Does
✅ Creates unified deprecation system with version tracking
✅ Adds comprehensive MIGRATION.md with clear upgrade guidance
✅ Updates 7 key files to use the new system
✅ Ensures backward compatibility - zero breaking changes
✅ Adds future CI integration hooks for expired deprecation detection
Key Components
1. Centralized Deprecation System (
praisonaiagents/utils/deprecation.py)@deprecateddecorator withsinceandremovalversion trackingwarn_deprecated_param()for consistent parameter warnings2. Comprehensive Documentation (
MIGRATION.md)3. Updated Files
agent/agent.py- 7 parameter deprecations (llm → model, allow_delegation → handoffs, etc.)task/task.py- 2 parameter deprecations (callback → on_task_complete, guardrail → guardrails)agent/autonomy.py- Class deprecations (AutonomySignal → EscalationSignal)paths.py- Directory migration warningsprocess/process.py- Workflow API deprecationbots/_approval.py- Module-level deprecationllm/__init__.py- Function deprecationBefore vs After
❌ Before: Inconsistent deprecation warnings
✅ After: Unified system with version tracking
Deprecation Timeline
Testing
Future CI Integration
The system includes
check_expired_deprecations()for CI workflows to automatically detect when deprecations should be removed based on version comparisons.Fixes #1135
🤖 Generated with Claude Code
Summary by CodeRabbit
Documentation
Deprecations