fix: resolve cli_backend validation, bot streaming, and tool resolution regressions#2004
fix: resolve cli_backend validation, bot streaming, and tool resolution regressions#2004cursor[bot] wants to merge 1 commit into
Conversation
… changes - Pass adapter.name (not adapter object) to cli_backend validation so praisonai YAML configs with cli_backend/runtime no longer always fail - Bridge bot streaming callbacks via stream_emitter and pass cancel_token to astart(), fixing TypeError on first streamed message and /stop handling - Import ToolResolver from praisonai.tool_resolver in bot defaults so configured tools are not silently dropped to the fallback set Adds regression tests covering all three fixes. Co-authored-by: Mervin Praison <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? |
✅ Action performedReview finished.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThree bug fixes are applied: ChangesThree Bug Fixes and Regression Tests
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 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)
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 |
|
@copilot Do a thorough review of this PR. Read ALL existing reviewer comments above from Qodo, Coderabbit, and Gemini first — incorporate their findings. Review areas:
|
|
@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 |
|
Claude finished @MervinPraison's task —— View job ✅ Architecture Review CompleteReview Checklist:
📋 Review SummaryI've thoroughly reviewed all changes and comments from other reviewers (Qodo, CodeRabbit). This PR fixes three critical regressions with minimal, focused changes. 🔍 Detailed Analysis1. CLI Backend Validation Fix (
|
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? |
Greptile SummaryThis PR fixes three independent regressions: a type mismatch that caused CLI-backend validation to always fail, a
Confidence Score: 4/5All three fixes are targeted and low-risk; the streaming change is consistent with the existing pattern in gateway/server.py. Each fix is a small, well-scoped change. The
Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant Bot as Bot Handler
participant BSM as BotSessionManager.chat()
participant Emitter as agent.stream_emitter
participant Agent as agent.astart()
participant CB as stream_callback
Bot->>BSM: chat(agent, user_id, prompt, stream_callback)
BSM->>Emitter: getattr(agent, "stream_emitter", None)
alt emitter is not None
BSM->>Emitter: add_callback(bridged_stream_callback)
end
BSM->>Agent: "astart(prompt, stream=True, cancel_token=controller)"
Agent-->>Emitter: emit(event) [during streaming]
Emitter-->>CB: bridged_stream_callback(event) → stream_callback(event)
Agent-->>BSM: response
BSM->>Emitter: remove_callback(bridged_stream_callback)
BSM-->>Bot: response
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant Bot as Bot Handler
participant BSM as BotSessionManager.chat()
participant Emitter as agent.stream_emitter
participant Agent as agent.astart()
participant CB as stream_callback
Bot->>BSM: chat(agent, user_id, prompt, stream_callback)
BSM->>Emitter: getattr(agent, "stream_emitter", None)
alt emitter is not None
BSM->>Emitter: add_callback(bridged_stream_callback)
end
BSM->>Agent: "astart(prompt, stream=True, cancel_token=controller)"
Agent-->>Emitter: emit(event) [during streaming]
Emitter-->>CB: bridged_stream_callback(event) → stream_callback(event)
Agent-->>BSM: response
BSM->>Emitter: remove_callback(bridged_stream_callback)
BSM-->>Bot: response
Reviews (1): Last reviewed commit: "fix: resolve three critical regressions ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if stream_callback: | ||
| # Streaming path: use agent.astart() with stream callback and timeout | ||
| # Streaming path: bridge events via stream_emitter because | ||
| # achat()/astart() do not accept stream_callback directly. | ||
| emitter = getattr(agent, "stream_emitter", None) | ||
| if emitter is not None: | ||
| def bridged_stream_callback(event): | ||
| try: | ||
| result = stream_callback(event) | ||
| if asyncio.iscoroutine(result): | ||
| asyncio.get_running_loop().create_task(result) | ||
| except Exception as cb_exc: | ||
| logger.warning("Stream callback failed: %s", cb_exc) | ||
|
|
||
| emitter.add_callback(bridged_stream_callback) |
There was a problem hiding this comment.
stream_emitter is absent
When stream_callback is provided but getattr(agent, "stream_emitter", None) returns None, the callback is never registered yet astart(stream=True) is still called. The original caller's streaming callback is silently ignored with no log warning, making it hard to diagnose why no stream events arrive. A logger.warning when emitter is None would make this detectable in production.
| class TestBotDefaultToolResolution: | ||
| """Bot defaults must resolve tools via praisonai.tool_resolver.ToolResolver.""" | ||
|
|
||
| def test_resolve_tool_names_uses_praisonai_tool_resolver(self): | ||
| from praisonai.bots._defaults import _resolve_tool_names_with_workspace | ||
|
|
||
| sentinel = object() | ||
| with patch( | ||
| "praisonaiagents.tools.profiles.resolve_profiles", | ||
| return_value=[sentinel], | ||
| ) as mock_profiles: | ||
| resolved = _resolve_tool_names_with_workspace(["search_web"]) | ||
|
|
||
| assert sentinel in resolved | ||
| mock_profiles.assert_called_once_with("web") |
There was a problem hiding this comment.
ToolResolver code path is not exercised
The test only passes ["search_web"], which maps through profile_map and exits via resolve_profiles before ToolResolver is ever instantiated. The import of ToolResolver is validated implicitly (an ImportError would surface), but resolver.resolve_many([...]) for a non-profile tool is never exercised. Adding a second assertion with a tool name that falls through to individual_tools (e.g. "my_custom_tool") would cover the full intended fix path.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
2 participants
Summary
Fixes three critical regressions found in the 2026-06-18 scheduled bug scan.
Bugs fixed
1. cli_backend validation always fails (PR #1988 regression)
cli_backendorruntimeonframework: praisonairaisedValueError, blocking all CLI-backend workflows._prepare_for_run()passed the adapter object to_validate_cli_backend_compatibility(), which comparedframework != 'praisonai'— always true for an object.adapter.nameinstead.2. Progressive streaming bots crash on first message
streaming=TruehitTypeError: achat() got an unexpected keyword argument 'stream_callback'on every message._session.pypassedstream_callbackdirectly toagent.astart(), butachat()does not accept that parameter.agent.stream_emitter.add_callback()and callastart(stream=True, cancel_token=controller).3. Bot default tools silently dropped
clarify,todo_*, file tools, etc.)._defaults.pyimported non-existentToolResolverfrompraisonaiagents.tools;AttributeErrorwas caught and returned[].praisonai.tool_resolver.Validation
tests/unit/test_critical_bug_fixes_20260618.py(4 tests)Summary by CodeRabbit
Bug Fixes
Tests