feat: model-aware runtime profiles (system-prompt & edit-format tuning)#2867
feat: model-aware runtime profiles (system-prompt & edit-format tuning)#2867praisonai-triage-agent[bot] wants to merge 2 commits into
Conversation
Introduces an opt-in, protocol-driven RuntimeProfile registry in core (praisonaiagents/runtime/profiles.py) that maps a model family to system-prompt segment overrides and a preferred edit-tool format. _build_system_prompt now consults the resolved profile; the behaviour-neutral "default" profile keeps generated prompts byte-for-byte identical when no profile is configured. Co-authored-by: MervinPraison <MervinPraison@users.noreply.github.com>
|
@coderabbitai review |
|
/review |
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
MervinPraison
added
pipeline/blocked:ci
✅ Action performedReview finished.
|
Greptile SummaryThis PR adds model-aware runtime profiles for agents. The main changes are:
Confidence Score: 4/5This is close, but these issues should be fixed before merging.
src/praisonai-agents/praisonaiagents/runtime/profiles.py; src/praisonai-agents/praisonaiagents/agent/chat_mixin.py Important Files Changed
Reviews (2): Last reviewed commit: "fix: harden runtime profiles per reviewe..." | Re-trigger Greptile |
📝 WalkthroughWalkthroughThis PR introduces an opt-in, model-aware "runtime profiles" system in praisonaiagents. A new ChangesModel-aware runtime profiles
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Agent
participant ChatMixin
participant resolve_profile
participant RuntimeProfileRegistry
participant RuntimeProfile
Agent->>Agent: __init__(runtime_profile=...)
Agent->>ChatMixin: _build_system_prompt(tools)
ChatMixin->>ChatMixin: _resolve_runtime_profile()
ChatMixin->>resolve_profile: resolve_profile(model, name)
resolve_profile->>RuntimeProfileRegistry: resolve(model, name)
RuntimeProfileRegistry-->>resolve_profile: RuntimeProfile instance
resolve_profile-->>ChatMixin: RuntimeProfile instance
ChatMixin->>RuntimeProfile: apply_system_prompt(base_prompt)
RuntimeProfile-->>ChatMixin: overridden or unchanged prompt
ChatMixin-->>Agent: final system prompt
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ 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 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
@claude You are the FINAL architecture reviewer. If the branch is under MervinPraison/PraisonAI (not a fork), you are able to make modifications to this branch and push directly. SCOPE: Focus ONLY on Python packages (praisonaiagents, praisonai). Do NOT modify praisonai-rust or praisonai-ts. Read ALL comments above from Gemini, Qodo, CodeRabbit, and Copilot carefully before responding. Phase 1: Review per AGENTS.md
Phase 2: FIX Valid Issues Phase 3: Final Verdict |
MervinPraison
added
pipeline/awaiting-merge-gate
|
Claude finished @MervinPraison's task —— View job Final Architecture Review — PR #2867
Phase 1 — Architecture per AGENTS.md
Phase 2 — Fixes pushed (commit
|
- Protect built-in "default" profile from entry-point override so unconfigured agents keep byte-for-byte identical prompts (Greptile P1) - Surface invalid explicit runtime_profile dicts via RuntimeProfile.from_dict key validation instead of silently swallowing (Greptile P2) - Fix global registry state leak in test_global_register_profile (Greptile P2) - Add tests for from_dict validation Co-authored-by: Mervin Praison <MervinPraison@users.noreply.github.com>
MervinPraison
added
pipeline/blocked:stale-final
| with self._lock: | ||
| if not override and name in self._profiles: | ||
| raise ValueError(f"Profile '{name}' is already registered") | ||
| self._profiles[name] = profile |
There was a problem hiding this comment.
The discovery path skips entry points named default, but the public API can still replace the reserved default profile through register_profile("default", profile). After that, an agent with runtime_profile=None and an unknown model resolves the replaced default profile and applies its prompt prefix or suffix. That changes prompts for callers who did not opt in, so the compatibility guarantee is still reachable through the registry API.
| return RuntimeProfile.from_dict(configured) | ||
|
|
||
| model = self.llm if isinstance(self.llm, str) else str(self.llm) if self.llm else None | ||
| if isinstance(configured, str): |
There was a problem hiding this comment.
Explicit string profile names still use the same fallback path as implicit model-family resolution. For example, Agent(runtime_profile="claude") resolves the default profile when no profile with that exact name is registered, with no error or warning. That silently drops a required explicit setting instead of failing when the configured profile name is wrong.
![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
🧹 Nitpick comments (4)
src/praisonai-agents/tests/unit/runtime/test_profiles.py (1)
30-33: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueMissing test coverage for
o3model family.
resolve_model_familychecksm.startswith("o3")but no test exercises this branch. Adding a case likeresolve_model_family("o3-mini") == "openai"would close the gap.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/praisonai-agents/tests/unit/runtime/test_profiles.py` around lines 30 - 33, Extend the test_openai function to cover the o3-specific branch in resolve_model_family by asserting that resolve_model_family("o3-mini") returns "openai".src/praisonai-agents/praisonaiagents/agent/agent.py (1)
537-537: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
clone_for_channeldoesn't propagateruntime_profile.
self.runtime_profileis stored as a new instance attribute, butclone_for_channel'sclone_kwargsdict (lines ~2259-2319) has noruntime_profileentry, so agents cloned for multi-channel gateway use silently revert to default profile resolution. (Note: the olderruntime=param has the same gap, so this may be a known limitation of that method rather than novel — but worth closing for the new feature too.)♻️ Suggested fix
'tool_search': getattr(self, '_tool_search_config', None), # Tool configuration - use consolidated config when available 'tool_config': getattr(self, '_tool_config', None), + + # Model-aware runtime profile + 'runtime_profile': getattr(self, 'runtime_profile', None),Also applies to: 1544-1546
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/praisonai-agents/praisonaiagents/agent/agent.py` at line 537, Update clone_for_channel’s clone_kwargs to include self.runtime_profile so cloned agents preserve the configured runtime profile; also propagate the legacy runtime value if it is stored separately, matching the existing constructor parameter names and clone behavior.src/praisonai-agents/praisonaiagents/runtime/profiles.py (1)
233-257: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winEntry-point discovery runs eagerly at import time and swallows failures silently.
Two related concerns in
_discover_entry_point_profiles:
- It's invoked unconditionally at module import (line 257), inconsistent with this file's own lazy
_ensure_builtin()pattern used for builtin profiles — every import ofprofiles.pypays theentry_points()metadata-scan cost even if no runtime-profile feature is ever used.importlib.metadata.entry_points()walks installed distribution metadata on each call, so this is worth deferring into the same lazy path as builtins.- The nested
except Exception: continue/except Exception: passblocks swallow all plugin load/discovery failures with no logging, matching Ruff's S110/S112/BLE001 hints — a broken third-party profile plugin fails completely silently.♻️ Suggested fix
- eps = entry_points(group="praisonaiagents.runtime_profiles") - for ep in eps: - try: - obj = ep.load() - profile = obj() if callable(obj) and not isinstance(obj, RuntimeProfile) else obj - if isinstance(profile, RuntimeProfile): - _global_registry.register(ep.name, profile) - except Exception: - # Plugins are optional; ignore individual discovery failures. - continue - except Exception: - # entry_points unavailable or errored; discovery is best-effort. - pass + eps = entry_points(group="praisonaiagents.runtime_profiles") + for ep in eps: + try: + obj = ep.load() + profile = obj() if callable(obj) and not isinstance(obj, RuntimeProfile) else obj + if isinstance(profile, RuntimeProfile): + _global_registry.register(ep.name, profile) + except Exception as e: + # Plugins are optional; ignore individual discovery failures. + logging.debug("Runtime profile plugin '%s' failed to load: %s", ep.name, e) + continue + except Exception as e: + # entry_points unavailable or errored; discovery is best-effort. + logging.debug("Runtime profile entry-point discovery skipped: %s", e)And call
_discover_entry_point_profiles()from within_ensure_builtin()(guarded by the same flag) instead of at module scope.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/praisonai-agents/praisonaiagents/runtime/profiles.py` around lines 233 - 257, Defer entry-point discovery until the existing lazy initialization path by removing the module-level _discover_entry_point_profiles() call and invoking it from _ensure_builtin() under the same one-time guard. Replace the broad silent exception handlers in _discover_entry_point_profiles with appropriate logging that includes the failing entry point or discovery operation and exception details, while continuing to isolate individual plugin failures.Source: Linters/SAST tools
src/praisonai-agents/praisonaiagents/agent/chat_mixin.py (1)
269-292: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winExplicit misconfiguration is silently swallowed the same way as ordinary resolution fallback.
_resolve_runtime_profile()is called inside_build_system_prompt's broadtry/except Exception(lines 255-264). That's appropriate for automatic model-family resolution (should never break chat), but it also masks genuine user errors for explicit configuration — e.g.RuntimeProfile(**configured)raisingTypeErroron an unknown dict key, or an unrecognizedruntime_profiletype (e.g. an int) silently falling through toresolve_profile(model=model)at line 291 instead of raising. A user who misconfiguresruntime_profile=gets no signal beyond a debug log that most deployments won't see, and the feature will just quietly no-op forever.Consider validating explicit
dict/unsupported-type inputs eagerly (e.g. inAgent.__init__or at the top of_resolve_runtime_profile) and raising a clearValueError/TypeError, while keeping the broad catch only around the automatic model-basedresolve_profile()fallback.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/praisonai-agents/praisonaiagents/agent/chat_mixin.py` around lines 269 - 292, Explicitly validate runtime_profile inputs in _resolve_runtime_profile before automatic resolution: preserve None, False, RuntimeProfile, valid strings, and dictionaries that successfully construct RuntimeProfile, but raise a clear TypeError or ValueError for invalid dictionaries or unsupported types instead of falling through to resolve_profile(model=model). Keep exception swallowing limited to automatic model-based resolve_profile() fallback in _build_system_prompt.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/praisonai-agents/praisonaiagents/agent/chat_mixin.py`:
- Around line 251-267: Ensure runtime profile application also occurs when
_build_system_prompt() returns a cached prompt. Refactor the cache-hit path and
the existing profile logic around _resolve_runtime_profile() so cached_prompt is
passed through profile.apply_system_prompt() before returning, while preserving
error handling and avoiding duplicate application on uncached prompts.
In `@src/praisonai-agents/tests/unit/runtime/test_profiles.py`:
- Around line 123-125: Add an autouse pytest fixture in the runtime profile
tests that clears the global profile registry before and after each test, using
the registry’s clear() method. Ensure test_global_register_profile and all other
tests share this cleanup so registered profiles cannot leak across the test
session.
---
Nitpick comments:
In `@src/praisonai-agents/praisonaiagents/agent/agent.py`:
- Line 537: Update clone_for_channel’s clone_kwargs to include
self.runtime_profile so cloned agents preserve the configured runtime profile;
also propagate the legacy runtime value if it is stored separately, matching the
existing constructor parameter names and clone behavior.
In `@src/praisonai-agents/praisonaiagents/agent/chat_mixin.py`:
- Around line 269-292: Explicitly validate runtime_profile inputs in
_resolve_runtime_profile before automatic resolution: preserve None, False,
RuntimeProfile, valid strings, and dictionaries that successfully construct
RuntimeProfile, but raise a clear TypeError or ValueError for invalid
dictionaries or unsupported types instead of falling through to
resolve_profile(model=model). Keep exception swallowing limited to automatic
model-based resolve_profile() fallback in _build_system_prompt.
In `@src/praisonai-agents/praisonaiagents/runtime/profiles.py`:
- Around line 233-257: Defer entry-point discovery until the existing lazy
initialization path by removing the module-level
_discover_entry_point_profiles() call and invoking it from _ensure_builtin()
under the same one-time guard. Replace the broad silent exception handlers in
_discover_entry_point_profiles with appropriate logging that includes the
failing entry point or discovery operation and exception details, while
continuing to isolate individual plugin failures.
In `@src/praisonai-agents/tests/unit/runtime/test_profiles.py`:
- Around line 30-33: Extend the test_openai function to cover the o3-specific
branch in resolve_model_family by asserting that resolve_model_family("o3-mini")
returns "openai".
🪄 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: 081da02d-95b4-42f4-a307-138b55217c29
📒 Files selected for processing (5)
src/praisonai-agents/praisonaiagents/agent/agent.pysrc/praisonai-agents/praisonaiagents/agent/chat_mixin.pysrc/praisonai-agents/praisonaiagents/runtime/__init__.pysrc/praisonai-agents/praisonaiagents/runtime/profiles.pysrc/praisonai-agents/tests/unit/runtime/test_profiles.py
| # Apply model-aware runtime profile overrides (opt-in). When no profile | ||
| # is configured for the active model family, the resolved profile is the | ||
| # behaviour-neutral "default" whose apply_system_prompt is a no-op, so | ||
| # the generated prompt is byte-for-byte identical to before. | ||
| try: | ||
| profile = self._resolve_runtime_profile() | ||
| if profile is not None: | ||
| # apply_system_prompt is a pure no-op unless the profile declares | ||
| # prompt overrides, so the default/family profiles leave the | ||
| # generated prompt byte-for-byte identical. | ||
| system_prompt = profile.apply_system_prompt(system_prompt) | ||
| except Exception as e: | ||
| import logging | ||
| logging.debug(f"Runtime profile application failed: {e}", exc_info=True) | ||
|
|
||
| # Note: Caching is done BEFORE session context injection to avoid cross-user leakage | ||
| return system_prompt |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the file and inspect the relevant function shape first.
git ls-files 'src/praisonai-agents/praisonaiagents/agent/chat_mixin.py' 'src/praisonai-agents/praisonaiagents/**/test*.py' 'tests/**' | sed -n '1,120p'
echo '--- outline ---'
ast-grep outline src/praisonai-agents/praisonaiagents/agent/chat_mixin.py --view expanded
echo '--- relevant slice ---'
python3 - <<'PY'
from pathlib import Path
p = Path('src/praisonai-agents/praisonaiagents/agent/chat_mixin.py')
lines = p.read_text().splitlines()
for start, end in [(1, 340)]:
for i in range(start-1, min(end, len(lines))):
if 60 <= i+1 <= 290:
print(f"{i+1:4}: {lines[i]}")
PYRepository: MervinPraison/PraisonAI
Length of output: 19502
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for runtime profile, system prompt cache, and related tests.
rg -n "runtime_profile|_system_prompt_cache|_build_system_prompt|apply_system_prompt|Session context is per-turn" src/praisonai-agents/praisonaiagents -S
echo '--- tests mentioning runtime profile ---'
rg -n "RuntimeProfile|runtime profile|apply_system_prompt|system prompt cache" src/praisonai-agents -SRepository: MervinPraison/PraisonAI
Length of output: 13488
Runtime profiles must run on cache hits too src/praisonai-agents/praisonaiagents/agent/chat_mixin.py:85-87,251-261
_build_system_prompt() returns cached_prompt before the new runtime_profile override runs, so any configured profile only affects the first turn for non-memory agents. Apply the profile before returning the cached value as well.
🧰 Tools
🪛 Ruff (0.15.20)
[warning] 262-262: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/praisonai-agents/praisonaiagents/agent/chat_mixin.py` around lines 251 -
267, Ensure runtime profile application also occurs when _build_system_prompt()
returns a cached prompt. Refactor the cache-hit path and the existing profile
logic around _resolve_runtime_profile() so cached_prompt is passed through
profile.apply_system_prompt() before returning, while preserving error handling
and avoiding duplicate application on uncached prompts.
| def test_global_register_profile(self): | ||
| register_profile("global-test", RuntimeProfile(name="global-test", system_prompt_prefix="G")) | ||
| assert resolve_profile(name="global-test").name == "global-test" |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Global registry pollution: test_global_register_profile never cleans up.
This test registers "global-test" on the global singleton registry but never removes it. The profile persists for all subsequent tests in the session. While current tests use in checks (not exact-match), any future test asserting the exact profile set will fail non-deterministically depending on test ordering. The registry exposes clear() specifically for this purpose.
Add an autouse fixture to reset the global registry between tests:
🧹 Suggested fix: autouse fixture for global registry cleanup
+import pytest
+from praisonaiagents.runtime.profiles import (
+ RuntimeProfile,
+ RuntimeProfileProtocol,
+ RuntimeProfileRegistry,
+ resolve_model_family,
+ resolve_profile,
+ register_profile,
+ list_profiles,
+ DEFAULT_PROFILE_NAME,
+)
+
+
+@pytest.fixture(autouse=True)
+def _reset_global_registry():
+ """Reset the global profile registry before and after each test."""
+ # Import the global registry instance used by the module-level functions
+ from praisonaiagents.runtime.profiles import _global_registry
+ _global_registry.clear()
+ yield
+ _global_registry.clear()If the global registry variable name differs, adjust accordingly. The key point is ensuring test isolation by clearing the global registry around each test.
📝 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.
| def test_global_register_profile(self): | |
| register_profile("global-test", RuntimeProfile(name="global-test", system_prompt_prefix="G")) | |
| assert resolve_profile(name="global-test").name == "global-test" | |
| import pytest | |
| from praisonaiagents.runtime.profiles import ( | |
| RuntimeProfile, | |
| RuntimeProfileProtocol, | |
| RuntimeProfileRegistry, | |
| resolve_model_family, | |
| resolve_profile, | |
| register_profile, | |
| list_profiles, | |
| DEFAULT_PROFILE_NAME, | |
| ) | |
| `@pytest.fixture`(autouse=True) | |
| def _reset_global_registry(): | |
| """Reset the global profile registry before and after each test.""" | |
| from praisonaiagents.runtime.profiles import _global_registry | |
| _global_registry.clear() | |
| yield | |
| _global_registry.clear() |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/praisonai-agents/tests/unit/runtime/test_profiles.py` around lines 123 -
125, Add an autouse pytest fixture in the runtime profile tests that clears the
global profile registry before and after each test, using the registry’s clear()
method. Ensure test_global_register_profile and all other tests share this
cleanup so registered profiles cannot leak across the test session.
praisonai-triage-agent
Bot
removed
pipeline/blocked:ci
1 participant
Fixes #2863
Summary
Adds an opt-in, protocol-driven runtime profile registry to core (
praisonaiagents) that maps a model family (GPT / Claude / Gemini) to a system-prompt profile and a preferred edit-tool format. Today's behaviour is thedefaultprofile, so with no profile configured the generated prompt and advertised tools are byte-for-byte unchanged.Changes
praisonaiagents/runtime/profiles.pyRuntimeProfileProtocol(core protocol)RuntimeProfiledataclass (system_prompt_prefix/suffix,preferred_edit_format,extras)RuntimeProfileRegistrymirroringruntime/registry.pyregister_profile/resolve_profile/list_profiles/resolve_model_familydefault,anthropic,openai,gemini) — family profiles carry only a preferred edit format and are prompt-neutral by defaultpraisonaiagents.runtime_profilesruntime/__init__.py: lazy exports for the new APIagent/chat_mixin.py::_build_system_prompt: applies the resolved profile at the end (pure no-op unless the profile declares prompt overrides); adds_resolve_runtime_profile()honouringAgent(runtime_profile=...)(bool/str/dict/RuntimeProfile)agent/agent.py: adds theruntime_profileparam (+4 lines only; no bloat)Backward compatibility
defaultprofile'sapply_system_promptis a pure no-op.preferred_edit_format(no prompt text) → prompt output is identical to today.Tests
tests/unit/runtime/test_profiles.py— 22 tests covering family resolution, prompt application, registry behaviour, and backward compatibility. All pass.test_resolve.py/test_resolver.py/test_turn_context.pyare unrelated (verified against baseline before changes).Generated with Claude Code
Summary by CodeRabbit
New Features
Compatibility