fix: add fail-fast YAML schema validation with actionable errors#2128
Conversation
|
@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.
|
|
Important Review skippedBot user detected. To trigger a single review, invoke the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughIntroduces a Pydantic-based YAML configuration validation system. A new ChangesPydantic YAML Validation System
Sequence Diagram(s)sequenceDiagram
actor User
participant CLI as praisonai validate
participant ConfigValidator
participant YAMLConfig as YAMLConfig (Pydantic)
participant ToolResolver
User->>CLI: praisonai validate agents.yaml --strict
CLI->>ConfigValidator: validate_yaml_file("agents.yaml", strict=True)
ConfigValidator->>ConfigValidator: yaml.safe_load(file)
ConfigValidator->>YAMLConfig: model_validate(config_dict)
YAMLConfig-->>ConfigValidator: validated model / ValidationError list
ConfigValidator->>YAMLConfig: validate_cross_references()
YAMLConfig-->>ConfigValidator: cross-reference error list
ConfigValidator->>ToolResolver: resolve_tool(name) per referenced tool
ToolResolver-->>ConfigValidator: resolved / unknown / optional warning
ConfigValidator->>ConfigValidator: _check_unknown_fields() → warnings → errors (strict)
ConfigValidator-->>CLI: ValidationResult(valid, errors, warnings)
CLI-->>User: exit 0 (valid) or exit 1 + error list
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
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 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:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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/praisonai/cli/commands/validate.py`:
- Line 12: The validation command exits early when a file is missing before
checking if the --json flag is set, causing it to emit formatted text via rich
print instead of structured JSON output. Reorganize the code to ensure the file
existence pre-check occurs after the JSON flag is evaluated, or wrap the file
missing failure with conditional logic that outputs JSON when --json is enabled
and formatted text otherwise. This applies to the pre-check logic and the error
handling branches around lines 62-67 and 75-85 in addition to the initial
imports.
In `@src/praisonai/praisonai/config/schema.py`:
- Around line 330-343: The handoff target validation in the loop starting with
iterating through agents_dict is checking if targets exist in all_roles while
simultaneously populating all_roles in the same pass. This causes forward
references to be incorrectly flagged as undefined when an agent's handoff
references a role defined later in the configuration. To fix this, separate the
logic into two passes: first iterate through all agents_dict values to
completely populate the all_roles set, then in a second pass iterate again to
validate that all handoff targets in agent_config.handoff.to exist in the
now-complete all_roles set.
- Around line 193-206: The validate_step_type method checks for known step types
like 'task', 'parallel', 'loop', and 'route' but lacks validation to reject
unknown or unsupported step types, allowing invalid configurations to pass
silently. Add an else clause at the end of the conditional chain that raises a
ValueError when the step type does not match any of the supported types,
including the step name and the invalid type in the error message to provide
clear context during debugging.
- Around line 116-123: The handle_stream_alias validator in the stream field
validator only processes the stream field value but does not update the
streaming field itself, so backward-compatible configs using stream still result
in streaming being False. Replace or supplement this single field_validator with
a model_validator that runs after all fields are populated, checking if stream
is provided and streaming is at its default value, then explicitly setting
streaming to match the stream value. This ensures that providing stream: true
actually enables the streaming functionality as intended.
In `@src/praisonai/praisonai/config/validator.py`:
- Around line 211-245: The _is_known_optional_tool method in the validator.py
file is too permissive because it returns True for any tool name ending with
'Tool', even if it's not in the known_optional set. This masks typos and unknown
tools by treating them as optional dependencies instead of failing validation.
Remove the or tool_name.endswith('Tool') fallback condition from the return
statement in _is_known_optional_tool so that only tools explicitly listed in the
known_optional set are recognized as optional tools, allowing unknown tool names
to fail validation properly.
- Around line 85-103: The validate_config method does not validate that the
config parameter is a dictionary before attempting to unpack it with `**config`
into YAMLConfig. Non-mapping YAML input (None, lists, or scalar values) will
raise a TypeError that bypasses the aggregated error reporting mechanism. Add a
type check before the try block that instantiates YAMLConfig to verify that
config is a dictionary; if it is not, append an error message to the errors list
using the file_prefix format and continue with the validation, ensuring a proper
ValidationResult is always returned instead of raising an unhandled exception.
🪄 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: 31e6f1c1-edfc-420e-9bb3-a23136b1c4f8
📒 Files selected for processing (7)
src/praisonai/praisonai/agents_generator.pysrc/praisonai/praisonai/cli/app.pysrc/praisonai/praisonai/cli/commands/validate.pysrc/praisonai/praisonai/config/__init__.pysrc/praisonai/praisonai/config/schema.pysrc/praisonai/praisonai/config/validator.pysrc/praisonai/pyproject.toml
| from typing import Optional | ||
|
|
||
| import typer | ||
| from rich import print |
There was a problem hiding this comment.
--json mode should emit JSON for missing-file failures too.
The pre-check exits before the JSON branch, so --json returns formatted text instead of structured output on a missing file. This breaks CI parsing for one of the most common failure cases.
Suggested fix
@@
-import typer
-from rich import print
+import typer
from rich.console import Console
from rich.table import Table
@@
- # Check if file exists
- file_path = Path(file)
- if not file_path.exists():
- console.print(f"[red]✗ File not found:[/red] {file}", style="bold")
- sys.exit(1)
+ file_path = Path(file)
@@
if json_output:
import json
output = {
"file": str(file_path),
"valid": result.valid,
"errors": result.errors,
"warnings": result.warnings,
"strict_mode": strict
}
- print(json.dumps(output, indent=2))
+ sys.stdout.write(json.dumps(output, indent=2) + "\n")
sys.exit(0 if result.valid else 1)Also applies to: 62-67, 75-85
🧰 Tools
🪛 Ruff (0.15.17)
[error] 12-12: Import print is shadowing a Python builtin
(A004)
🤖 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/praisonai/cli/commands/validate.py` at line 12, The validation
command exits early when a file is missing before checking if the --json flag is
set, causing it to emit formatted text via rich print instead of structured JSON
output. Reorganize the code to ensure the file existence pre-check occurs after
the JSON flag is evaluated, or wrap the file missing failure with conditional
logic that outputs JSON when --json is enabled and formatted text otherwise.
This applies to the pre-check logic and the error handling branches around lines
62-67 and 75-85 in addition to the initial imports.
| @field_validator('stream') | ||
| @classmethod | ||
| def handle_stream_alias(cls, v, info): | ||
| """Handle stream as alias for streaming.""" | ||
| if v is not None and 'streaming' not in info.data: | ||
| return v | ||
| return info.data.get('streaming', False) if v is None else v | ||
|
|
There was a problem hiding this comment.
stream alias does not actually enable streaming.
stream: true is stored on stream, but streaming remains its default (False), so backward-compatible configs lose intended behavior.
Suggested fix
- `@field_validator`('stream')
- `@classmethod`
- def handle_stream_alias(cls, v, info):
- """Handle stream as alias for streaming."""
- if v is not None and 'streaming' not in info.data:
- return v
- return info.data.get('streaming', False) if v is None else v
+ `@model_validator`(mode='before')
+ `@classmethod`
+ def normalize_stream_alias(cls, data):
+ """Map legacy 'stream' into canonical 'streaming'."""
+ if isinstance(data, dict) and 'streaming' not in data and 'stream' in data:
+ data['streaming'] = data['stream']
+ return data🤖 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/praisonai/config/schema.py` around lines 116 - 123, The
handle_stream_alias validator in the stream field validator only processes the
stream field value but does not update the streaming field itself, so
backward-compatible configs using stream still result in streaming being False.
Replace or supplement this single field_validator with a model_validator that
runs after all fields are populated, checking if stream is provided and
streaming is at its default value, then explicitly setting streaming to match
the stream value. This ensures that providing stream: true actually enables the
streaming functionality as intended.
| def validate_step_type(self): | ||
| """Validate step configuration based on type.""" | ||
| if self.type == 'task': | ||
| if not self.agent or not self.task: | ||
| raise ValueError(f"Task step '{self.name}' requires both 'agent' and 'task' fields") | ||
| elif self.type in ('parallel', 'loop'): | ||
| if not self.steps: | ||
| raise ValueError(f"{self.type.capitalize()} step '{self.name}' requires 'steps' field") | ||
| if self.type == 'loop' and self.count is None: | ||
| raise ValueError(f"Loop step '{self.name}' requires 'count' field") | ||
| elif self.type == 'route': | ||
| if not self.routes: | ||
| raise ValueError(f"Route step '{self.name}' requires 'routes' field") | ||
|
|
There was a problem hiding this comment.
Unknown workflow step types currently pass validation silently.
validate_step_type() checks known types but never rejects unsupported ones, so invalid configs bypass fail-fast validation.
Suggested fix
`@model_validator`(mode='after')
def validate_step_type(self):
"""Validate step configuration based on type."""
+ allowed = {'task', 'parallel', 'loop', 'route'}
+ if self.type not in allowed:
+ raise ValueError(
+ f"Step '{self.name}' has invalid type '{self.type}'. "
+ f"Allowed values: {', '.join(sorted(allowed))}"
+ )
+
if self.type == 'task':
if not self.agent or not self.task:
raise ValueError(f"Task step '{self.name}' requires both 'agent' and 'task' fields")🤖 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/praisonai/config/schema.py` around lines 193 - 206, The
validate_step_type method checks for known step types like 'task', 'parallel',
'loop', and 'route' but lacks validation to reject unknown or unsupported step
types, allowing invalid configurations to pass silently. Add an else clause at
the end of the conditional chain that raises a ValueError when the step type
does not match any of the supported types, including the step name and the
invalid type in the error message to provide clear context during debugging.
| all_roles = set() | ||
| for agents_dict in [self.roles, self.agents]: | ||
| if agents_dict: | ||
| for agent_config in agents_dict.values(): | ||
| all_roles.add(agent_config.role) | ||
|
|
||
| # Check handoff targets | ||
| if agent_config.handoff and isinstance(agent_config.handoff, HandoffConfig): | ||
| for target in agent_config.handoff.to: | ||
| if target not in all_roles: | ||
| errors.append( | ||
| f"Agent '{agent_config.role}' handoff references undefined role '{target}'. " | ||
| f"Available roles: {', '.join(sorted(all_roles))}" | ||
| ) |
There was a problem hiding this comment.
Handoff target validation is order-dependent and rejects forward references.
all_roles is populated and validated in the same pass, so a handoff to a role defined later can be incorrectly flagged as undefined.
Suggested fix
- all_roles = set()
- for agents_dict in [self.roles, self.agents]:
- if agents_dict:
- for agent_config in agents_dict.values():
- all_roles.add(agent_config.role)
-
- # Check handoff targets
- if agent_config.handoff and isinstance(agent_config.handoff, HandoffConfig):
- for target in agent_config.handoff.to:
- if target not in all_roles:
- errors.append(
- f"Agent '{agent_config.role}' handoff references undefined role '{target}'. "
- f"Available roles: {', '.join(sorted(all_roles))}"
- )
+ all_roles = set()
+ all_agent_configs = []
+ for agents_dict in [self.roles, self.agents]:
+ if agents_dict:
+ all_agent_configs.extend(agents_dict.values())
+ for agent_config in agents_dict.values():
+ all_roles.add(agent_config.role)
+
+ for agent_config in all_agent_configs:
+ if agent_config.handoff and isinstance(agent_config.handoff, HandoffConfig):
+ for target in agent_config.handoff.to:
+ if target not in all_roles:
+ errors.append(
+ f"Agent '{agent_config.role}' handoff references undefined role '{target}'. "
+ f"Available roles: {', '.join(sorted(all_roles))}"
+ )🤖 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/praisonai/config/schema.py` around lines 330 - 343, The handoff
target validation in the loop starting with iterating through agents_dict is
checking if targets exist in all_roles while simultaneously populating all_roles
in the same pass. This causes forward references to be incorrectly flagged as
undefined when an agent's handoff references a role defined later in the
configuration. To fix this, separate the logic into two passes: first iterate
through all agents_dict values to completely populate the all_roles set, then in
a second pass iterate again to validate that all handoff targets in
agent_config.handoff.to exist in the now-complete all_roles set.
| def validate_config(self, config: Dict[str, Any], strict: bool = False, file_path: Optional[str] = None) -> ValidationResult: | ||
| """Validate configuration dictionary. | ||
|
|
||
| Args: | ||
| config: Configuration dictionary | ||
| strict: If True, treat warnings as errors | ||
| file_path: Optional file path for better error messages | ||
|
|
||
| Returns: | ||
| ValidationResult with errors and warnings | ||
| """ | ||
| errors = [] | ||
| warnings = [] | ||
| file_prefix = f"{file_path}: " if file_path else "" | ||
|
|
||
| # Validate against schema | ||
| try: | ||
| yaml_config = YAMLConfig(**config) | ||
| except ValidationError as e: |
There was a problem hiding this comment.
Non-mapping YAML can crash validation before returning ValidationResult.
YAMLConfig(**config) assumes config is a dict. Empty YAML (None) or list/scalar YAML will raise TypeError and bypass aggregated error reporting.
Suggested fix
def validate_config(self, config: Dict[str, Any], strict: bool = False, file_path: Optional[str] = None) -> ValidationResult:
@@
errors = []
warnings = []
file_prefix = f"{file_path}: " if file_path else ""
+
+ if config is None:
+ return ValidationResult(
+ valid=False,
+ errors=[f"{file_prefix}Configuration is empty. Expected a YAML mapping at the root."]
+ )
+ if not isinstance(config, dict):
+ return ValidationResult(
+ valid=False,
+ errors=[f"{file_prefix}Invalid root type '{type(config).__name__}'. Expected a YAML mapping/object."]
+ )
# Validate against schema
try:
yaml_config = YAMLConfig(**config)📝 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 validate_config(self, config: Dict[str, Any], strict: bool = False, file_path: Optional[str] = None) -> ValidationResult: | |
| """Validate configuration dictionary. | |
| Args: | |
| config: Configuration dictionary | |
| strict: If True, treat warnings as errors | |
| file_path: Optional file path for better error messages | |
| Returns: | |
| ValidationResult with errors and warnings | |
| """ | |
| errors = [] | |
| warnings = [] | |
| file_prefix = f"{file_path}: " if file_path else "" | |
| # Validate against schema | |
| try: | |
| yaml_config = YAMLConfig(**config) | |
| except ValidationError as e: | |
| def validate_config(self, config: Dict[str, Any], strict: bool = False, file_path: Optional[str] = None) -> ValidationResult: | |
| """Validate configuration dictionary. | |
| Args: | |
| config: Configuration dictionary | |
| strict: If True, treat warnings as errors | |
| file_path: Optional file path for better error messages | |
| Returns: | |
| ValidationResult with errors and warnings | |
| """ | |
| errors = [] | |
| warnings = [] | |
| file_prefix = f"{file_path}: " if file_path else "" | |
| if config is None: | |
| return ValidationResult( | |
| valid=False, | |
| errors=[f"{file_prefix}Configuration is empty. Expected a YAML mapping at the root."] | |
| ) | |
| if not isinstance(config, dict): | |
| return ValidationResult( | |
| valid=False, | |
| errors=[f"{file_prefix}Invalid root type '{type(config).__name__}'. Expected a YAML mapping/object."] | |
| ) | |
| # Validate against schema | |
| try: | |
| yaml_config = YAMLConfig(**config) | |
| except ValidationError as e: |
🤖 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/praisonai/config/validator.py` around lines 85 - 103, The
validate_config method does not validate that the config parameter is a
dictionary before attempting to unpack it with `**config` into YAMLConfig.
Non-mapping YAML input (None, lists, or scalar values) will raise a TypeError
that bypasses the aggregated error reporting mechanism. Add a type check before
the try block that instantiates YAMLConfig to verify that config is a
dictionary; if it is not, append an error message to the errors list using the
file_prefix format and continue with the validation, ensuring a proper
ValidationResult is always returned instead of raising an unhandled exception.
| def _is_known_optional_tool(self, tool_name: str) -> bool: | ||
| """Check if a tool name is a known optional tool. | ||
|
|
||
| Args: | ||
| tool_name: Tool name to check | ||
|
|
||
| Returns: | ||
| True if it's a known optional tool | ||
| """ | ||
| # List of known optional tools that require extra dependencies | ||
| known_optional = { | ||
| # Database tools | ||
| 'PostgreSQLTool', 'MySQLTool', 'SQLiteTool', 'MongoDBTool', 'RedisTool', | ||
| 'SurrealDBTool', 'CassandraTool', 'ElasticsearchTool', | ||
|
|
||
| # Web/API tools | ||
| 'SlackTool', 'DiscordTool', 'TelegramTool', 'EmailTool', | ||
| 'TwitterTool', 'LinkedInTool', 'GitHubTool', | ||
|
|
||
| # Cloud tools | ||
| 'AWSTool', 'AzureTool', 'GCPTool', 'S3Tool', | ||
|
|
||
| # AI/ML tools | ||
| 'HuggingFaceTool', 'OpenAITool', 'AnthropicTool', | ||
|
|
||
| # Data tools | ||
| 'PandasTool', 'NumpyTool', 'ScipyTool', | ||
|
|
||
| # Other | ||
| 'BrowserTool', 'SeleniumTool', 'PlaywrightTool', | ||
| 'KubernetesTool', 'DockerTool', 'TerraformTool', | ||
| } | ||
|
|
||
| return tool_name in known_optional or tool_name.endswith('Tool') | ||
|
|
There was a problem hiding this comment.
endswith('Tool') turns unknown tool typos into warnings instead of errors.
Any unresolved tool named like SomethingTool is treated as “optional dependency,” which undermines fail-fast unknown-tool validation.
Suggested fix
- return tool_name in known_optional or tool_name.endswith('Tool')
+ return tool_name in known_optional🤖 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/praisonai/config/validator.py` around lines 211 - 245, The
_is_known_optional_tool method in the validator.py file is too permissive
because it returns True for any tool name ending with 'Tool', even if it's not
in the known_optional set. This masks typos and unknown tools by treating them
as optional dependencies instead of failing validation. Remove the or
tool_name.endswith('Tool') fallback condition from the return statement in
_is_known_optional_tool so that only tools explicitly listed in the
known_optional set are recognized as optional tools, allowing unknown tool names
to fail validation properly.
|
@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 & Fixes ✅I've completed the final architecture review and fixed all valid issues found by reviewers. Phase 1: Architecture Review ✅
Review findings:
Phase 2: Fixed Valid Issues ✅All 6 issues from CodeRabbit have been fixed:
Phase 3: Final Verdict ✅
Summary of Files ModifiedModified 3 files:
What was skipped:
Final Approval ✅This PR successfully implements comprehensive YAML validation with:
The PR is ready to merge. All validation features work correctly and the implementation follows the protocol-driven architecture principles. |
Greptile SummaryThis PR introduces Pydantic-based YAML schema validation with fail-fast behavior, aggregated error reporting, and a new
Confidence Score: 1/5Not safe to merge — the new validator is wired directly into the agent execution path and will crash every existing YAML configuration with a multi-word role name on startup. The
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[YAML File / Config Dict] --> B[validate_yaml_file / validate_config]
B --> C{Parse YAML}
C -->|YAMLError| D[Return invalid: YAML syntax error]
C -->|OK| E[YAMLConfig pydantic validation]
E -->|ValidationError| F[Return invalid: schema errors]
E -->|OK| G[validate_cross_references]
G --> H{Tasks → Agent refs valid?}
H -->|No| I[Collect errors]
H -->|Yes| J{Workflow steps → Agent refs valid?}
J -->|No| I
J -->|Yes| K{Handoff targets defined?}
K -->|No| I
K -->|Yes| L[_validate_tools]
I --> L
L --> M{Tool resolvable?}
M -->|No + known optional| N[Add warning]
M -->|No + unknown| O[Add error]
M -->|Yes| P[_check_unknown_fields]
N --> P
O --> P
P --> Q{strict mode?}
Q -->|Yes| R[Promote warnings to errors]
Q -->|No| S[Return ValidationResult]
R --> S
S --> T{result.valid?}
T -->|No| U[raise ValueError — agent run aborted]
T -->|Yes| V[Log warnings, continue]
%%{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"}}}%%
flowchart TD
A[YAML File / Config Dict] --> B[validate_yaml_file / validate_config]
B --> C{Parse YAML}
C -->|YAMLError| D[Return invalid: YAML syntax error]
C -->|OK| E[YAMLConfig pydantic validation]
E -->|ValidationError| F[Return invalid: schema errors]
E -->|OK| G[validate_cross_references]
G --> H{Tasks → Agent refs valid?}
H -->|No| I[Collect errors]
H -->|Yes| J{Workflow steps → Agent refs valid?}
J -->|No| I
J -->|Yes| K{Handoff targets defined?}
K -->|No| I
K -->|Yes| L[_validate_tools]
I --> L
L --> M{Tool resolvable?}
M -->|No + known optional| N[Add warning]
M -->|No + unknown| O[Add error]
M -->|Yes| P[_check_unknown_fields]
N --> P
O --> P
P --> Q{strict mode?}
Q -->|Yes| R[Promote warnings to errors]
Q -->|No| S[Return ValidationResult]
R --> S
S --> T{result.valid?}
T -->|No| U[raise ValueError — agent run aborted]
T -->|Yes| V[Log warnings, continue]
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
Automated merge readiness report (local git rebase origin/main scan, 2026-06-22 22:04 UTC) GitHub status
CI (key jobs: test-core, smoke, test-windows, main (3.11), review)
Rebase / CI notesGuidance
Final verdict after fix: reply merge-ready (CLEAN + green CI) or list remaining blockers. |
|
Claude finished @MervinPraison's task —— View job ✅ Merge Readiness ReportTask List
Fixed IssuesSuccessfully addressed all critical schema validation issues:
CI Status✅ Key CI jobs passing:
❌ Known pre-existing failure:
Files Modified
Final Verdict✅ READY TO MERGE The PR successfully implements fail-fast YAML schema validation with:
The schema validation now correctly handles dict-style LLM configurations, nested task definitions, and all common YAML patterns used in the codebase. |
|
Automated merge readiness report (local rebase, 2026-06-22 23:13 UTC) GitHub status
Conflict
Rebase onto main, keep fail-fast YAML validation (#2125), push --force-with-lease, confirm merge readiness. |
|
Claude finished @MervinPraison's task —— View job ✅ Merge Conflict Resolved SuccessfullyCompleted Tasks:
SummarySuccessfully rebased PR #2128 onto the latest main branch and resolved the merge conflict: Conflict Resolution:
Branch Status:
Merge Readiness: ✅ READYThe PR is now CLEAN and ready to merge. The fail-fast YAML schema validation is working correctly and all conflicts have been resolved. |
#2125) - Add comprehensive Pydantic-based schema validation for agents/tasks/workflow - Implement fail-fast validation with aggregated error reporting - Add cross-reference validation for agents, tasks, tools, and handoffs - Create 'praisonai validate' CLI command for pre-commit/CI validation - Update agents_generator to fail on validation errors (not just warn) - Add pydantic dependency for schema validation Key improvements: - Required fields are now enforced (role, goal, backstory) - Type checking for all configuration values - Cross-references validated (tasks->agents, tools->definitions, handoffs->roles) - Unknown fields detected with suggestions - Aggregated error messages show all problems at once - --strict mode treats warnings as errors Co-authored-by: MervinPraison <MervinPraison@users.noreply.github.com>
- Fix JSON output for missing files in validate command
- Fix stream alias not enabling streaming field (use model_validator before)
- Add validation to reject unknown workflow step types
- Fix handoff validation order-dependency (two-pass validation)
- Handle non-dict YAML input gracefully without crashing
- Remove overly permissive endswith('Tool') fallback
These fixes ensure fail-fast validation works correctly with proper error
aggregation and backward compatibility as intended.
- Change llm/function_calling_llm from Optional[str] to Optional[Union[str, Dict[str, Any]]] to support dict format with 'model' key - Add tasks field to AgentConfig to support nested task definitions - Add dependencies to known_top_level fields in validator - Add task normalization logic to convert dict tasks to TaskConfig objects These changes ensure backward compatibility with all existing YAML configurations while maintaining validation capabilities.
praisonai-triage-agent
Bot
force-pushed
the
claude/issue-2125-20260621-0842
branch
from
5c5d0e0 to
9d2b354
Compare
1 participant
Fixes #2125
Summary
Implements comprehensive YAML configuration validation with fail-fast behavior, aggregated error reporting, and a new 'praisonai validate' CLI command.
Changes
Schema Validation (New)
Cross-Reference Validation
Fail-Fast Behavior
CLI Command
Testing
Validated with test files containing various error conditions:
All errors are caught and reported with actionable messages.
Backward Compatibility
Summary by CodeRabbit
Release Notes
validateto check single YAML files,checkto scan directories, andschemato view configuration documentation.--strict) with JSON output support (--json).--quiet) available.