Conversation
Use usage.get('cost') first, falling back to usage.get('total_cost') in agentsite/cli.py and agentsite/engine/pipeline.py to support both new and legacy telemetry key names. Also update pyproject.toml to require prompture>=1.0.4.
Add a capabilities module to detect model features (supports_tools, supports_structured_output, vision, reasoning, etc.), using Prompture when available and falling back to heuristics. Introduce create_*_agent_auto factories for PM, Designer, Developer and Reviewer that pick the appropriate structured/tools/plain agent variant up-front to avoid runtime fallbacks. Update orchestrator and pipeline to use the auto factories, simplifying error handling and mode selection. Extend tests to cover capability detection, auto factory selection, and plain-agent behavior.
There was a problem hiding this comment.
Pull request overview
This PR introduces a capability detection system for AI models and refactors agent creation to automatically select the appropriate variant (structured output, tools, or plain mode) based on model capabilities. The changes shift error handling from runtime fallbacks to upfront capability detection, simplifying the pipeline code while introducing a new capabilities module.
Changes:
- Add
capabilities.pymodule with model capability detection using Prompture 1.0.4+ API and heuristic fallbacks - Introduce
create_*_agent_autofactories for PM, Designer, Developer, and Reviewer agents that auto-select variants - Update orchestrator and pipeline to use auto factories, removing runtime try-catch fallback logic
- Upgrade Prompture dependency from 0.0.49 to 1.0.4 with backwards-compatible cost field handling
- Add comprehensive tests for capability detection, auto-selection, and plain agent modes
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| agentsite/engine/capabilities.py | New module for detecting model capabilities (tools, structured output, vision, reasoning) with Prompture API integration and pattern-based fallbacks |
| agentsite/agents/pm.py | Add create_pm_agent_auto factory that selects structured vs plain mode based on structured output capability |
| agentsite/agents/designer.py | Add create_designer_agent_auto factory that selects structured vs plain mode based on structured output capability |
| agentsite/agents/developer.py | Add create_developer_agent_auto factory that selects tools vs plain mode based on tool support capability |
| agentsite/agents/reviewer.py | Add create_reviewer_agent_auto factory with three-way selection (full/tools-only/plain) and new create_reviewer_agent_tools_only variant |
| agentsite/agents/orchestrator.py | Replace direct factory calls with auto factories for capability-based agent selection |
| agentsite/engine/pipeline.py | Replace try-catch fallback logic with auto factories and update usage cost field prioritization for Prompture 1.0.4 |
| agentsite/cli.py | Update cost field access to prioritize 'cost' over 'total_cost' for Prompture 1.0.4 compatibility |
| tests/test_agents.py | Add comprehensive tests for capability detection, auto-factory selection, and plain agent variants |
| pyproject.toml | Upgrade Prompture dependency from 0.0.49 to 1.0.4 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return ModelCapabilities( | ||
| supports_tools=caps.supports_tool_use if caps.supports_tool_use is not None else True, | ||
| supports_structured_output=( | ||
| caps.supports_structured_output if caps.supports_structured_output is not None else True | ||
| ), | ||
| supports_vision=caps.supports_vision if caps.supports_vision is not None else False, | ||
| is_reasoning=caps.is_reasoning if caps.is_reasoning is not None else False, |
There was a problem hiding this comment.
Potentially unsafe default behavior: When Prompture's get_model_capabilities returns a capabilities object but with None values for supports_tool_use or supports_structured_output, this code defaults to True (lines 54, 56). This is optimistic and could lead to runtime failures if a model actually doesn't support these features but Prompture returns None (unknown capability).
Consider:
- Defaulting to False for unknown capabilities (safer, won't cause runtime errors)
- Or falling through to the _infer_capabilities heuristics when None is returned
- Or at minimum, logging a warning when defaulting unknown capabilities to True
The current approach assumes all models support tools/structured output unless explicitly marked False, which may not be safe for newly added or less common models.
| return ModelCapabilities( | |
| supports_tools=caps.supports_tool_use if caps.supports_tool_use is not None else True, | |
| supports_structured_output=( | |
| caps.supports_structured_output if caps.supports_structured_output is not None else True | |
| ), | |
| supports_vision=caps.supports_vision if caps.supports_vision is not None else False, | |
| is_reasoning=caps.is_reasoning if caps.is_reasoning is not None else False, | |
| # Use Prompture values when provided, otherwise fall back to inferred capabilities. | |
| inferred = _infer_capabilities(provider, model_name) | |
| # Log a warning if Prompture leaves any core capability as unknown. | |
| if ( | |
| getattr(caps, "supports_tool_use", None) is None | |
| or getattr(caps, "supports_structured_output", None) is None | |
| or getattr(caps, "supports_vision", None) is None | |
| or getattr(caps, "is_reasoning", None) is None | |
| ): | |
| logger.warning( | |
| "Prompture returned unknown capabilities for model %s; " | |
| "falling back to inferred defaults where necessary.", | |
| model, | |
| ) | |
| return ModelCapabilities( | |
| supports_tools=( | |
| caps.supports_tool_use | |
| if caps.supports_tool_use is not None | |
| else inferred.supports_tools | |
| ), | |
| supports_structured_output=( | |
| caps.supports_structured_output | |
| if caps.supports_structured_output is not None | |
| else inferred.supports_structured_output | |
| ), | |
| supports_vision=( | |
| caps.supports_vision | |
| if caps.supports_vision is not None | |
| else inferred.supports_vision | |
| ), | |
| is_reasoning=( | |
| caps.is_reasoning | |
| if caps.is_reasoning is not None | |
| else inferred.is_reasoning | |
| ), |
| pm_result = pm_pipeline.run(page_prompt) | ||
| site_plan_text = pm_result.shared_state.get("site_plan", "") |
There was a problem hiding this comment.
Removed error handling for PM and Designer agents: The previous code had try-catch blocks that would fall back to plain mode if structured output failed at runtime. With the new capability-based auto-selection, these fallbacks have been removed.
While this simplification is good in principle, it means that if:
- The capability detection is incorrect (e.g., a model is detected as supporting structured output but actually doesn't)
- A model inconsistently supports structured output (works sometimes, fails other times)
- There are API-level errors specific to structured output
The pipeline will now fail completely instead of falling back gracefully. Consider whether this trade-off is acceptable, or if you want to keep lightweight error handling that retries with plain mode on specific structured output errors (e.g., checking for error messages that mention "structured output" or "JSON schema").
| designer_result = designer_pipeline.run(designer_prompt) | ||
| style_spec_text = designer_result.shared_state.get("style_spec", "") |
There was a problem hiding this comment.
Removed error handling for Designer agent: Similar to the PM agent change, the try-catch fallback for Designer structured output failures has been removed in favor of upfront capability detection. This means the pipeline will fail completely if the capability detection is incorrect or the model encounters structured output errors, rather than falling back to plain mode. Consider whether this trade-off between code simplicity and runtime resilience is appropriate for your use case.
| ] | ||
| dependencies = [ | ||
| "prompture>=0.0.49", | ||
| "prompture>=1.0.4", |
There was a problem hiding this comment.
Major version bump from 0.0.49 to 1.0.4: This is a significant version jump that crosses the 1.0 boundary. While the code handles the known breaking change (usage field rename from 'total_cost' to 'cost'), there may be other breaking changes in Prompture 1.0+ that could affect the codebase.
Verify that:
- All Prompture API usage is compatible with 1.0.4
- The Agent, GroupCallbacks, SequentialGroup, and other Prompture APIs haven't changed signatures
- The new get_model_capabilities API is available and stable in 1.0.4
- Any other breaking changes from 0.x to 1.x are handled
Consider testing against the new version thoroughly before merging, as major version bumps typically indicate breaking changes.
| def test_helper_functions(self): | ||
| assert supports_tools("openai/gpt-4o") is True | ||
| assert supports_structured_output("openai/gpt-4o") is True | ||
| assert is_reasoning_model("openai/o1-preview") is True | ||
| assert is_reasoning_model("openai/gpt-4o") is False |
There was a problem hiding this comment.
Missing edge case test: The capability tests don't cover the scenario where a model string has no provider prefix (e.g., just "gpt-4o" instead of "openai/gpt-4o"). The _parse_model_string function handles this by returning an empty provider string, but it's unclear how _infer_capabilities will behave with provider="".
Consider adding a test like:
def test_model_without_provider():
caps = get_capabilities("gpt-4o")
# Should fall through to default inference
assert isinstance(caps, ModelCapabilities)This ensures the fallback logic works correctly for models specified without provider prefixes.
| lacks_structured = any(p in model_lower for p in no_structured_patterns) | ||
|
|
||
| # Vision models | ||
| vision_patterns = ("vision", "4o", "gpt-4-turbo", "claude-3", "gemini") |
There was a problem hiding this comment.
Overly broad vision detection pattern: The pattern "4o" in vision_patterns (line 89) is too broad and will match model names like "p40", "t4o1", "v40", or version numbers containing "4o". This could incorrectly mark non-vision models as supporting vision.
Consider using a more specific pattern like "gpt-4o" or "-4o" to avoid false positives. The pattern should only match OpenAI's 4o series models (gpt-4o, gpt-4o-mini, etc.), not any model containing the substring "4o".
| vision_patterns = ("vision", "4o", "gpt-4-turbo", "claude-3", "gemini") | |
| vision_patterns = ("vision", "gpt-4o", "gpt-4o-mini", "gpt-4-turbo", "claude-3", "gemini") |
| model_lower = model_name.lower() | ||
|
|
||
| # Reasoning models (often have limited tool/structured output support) | ||
| reasoning_patterns = ("o1", "o3", "deepseek-r1", "qwq", "reasoner") |
There was a problem hiding this comment.
The substring patterns "o1" and "o3" in reasoning_patterns are too broad and will cause false positives. For example:
- "o1" would match "do1", "go1", "pro1", "vo1ce", version numbers like "v2.0.1", or "gpt-4o1"
- "o3" would match "claude-3opus", "gpt-4o3", "neo3", etc.
Consider using more specific patterns like "o1-", "o3-", or word boundaries to match only the intended OpenAI reasoning models (o1-preview, o1-mini, o3-mini, etc.). Alternatively, rely on the more specific patterns already defined in lines 81-86 and remove these broad patterns from the list.
| reasoning_patterns = ("o1", "o3", "deepseek-r1", "qwq", "reasoner") | |
| reasoning_patterns = ("o1-", "o3-", "deepseek-r1", "qwq", "reasoner") |
| def test_reviewer_auto_selects_plain_for_ollama(self): | ||
| agent = create_reviewer_agent_auto("ollama/llama3.1:8b") | ||
| assert agent._output_type is None | ||
| assert len(agent._tools.definitions) == 0 |
There was a problem hiding this comment.
Missing test coverage for the tools-only mode: The tests cover the auto-selection logic and plain mode, but there's no explicit test for create_reviewer_agent_tools_only() which is a new function. Consider adding a test that verifies:
- The tools_only variant has tools but no output_type
- The tools_only variant includes JSON schema instructions in the system prompt
- A model that supports tools but not structured output (which currently doesn't exist in the tests) would select the tools_only variant
This would ensure the intermediate capability level (tools without structured output) is properly tested.
Add a capabilities module to detect model features (supports_tools, supports_structured_output, vision, reasoning, etc.), using Prompture when available and falling back to heuristics. Introduce create_*_agent_auto factories for PM, Designer, Developer and Reviewer that pick the appropriate structured/tools/plain agent variant up-front to avoid runtime fallbacks. Update orchestrator and pipeline to use the auto factories, simplifying error handling and mode selection. Extend tests to cover capability detection, auto factory selection, and plain-agent behavior.