diff --git a/crates/g3-core/src/prompts.rs b/crates/g3-core/src/prompts.rs index de3fb68..2d243d7 100644 --- a/crates/g3-core/src/prompts.rs +++ b/crates/g3-core/src/prompts.rs @@ -49,10 +49,10 @@ Each plan item MUST have: - `description`: What will be done - `state`: todo | doing | done | blocked - `touches`: Paths/modules this affects (forces \"where does this live?\") -- `checks`: Three required perspectives: +- `checks`: Required test perspectives: - `happy`: {desc, target} - Normal successful operation - - `negative`: {desc, target} - Error handling, invalid input - - `boundary`: {desc, target} - Edge cases, limits + - `negative`: [{desc, target}, ...] - Error handling, invalid input (>=1 required) + - `boundary`: [{desc, target}, ...] - Edge cases, limits (>=1 required) - `evidence`: (required when done) File:line refs, test names - `notes`: (required when done) Short implementation explanation @@ -79,11 +79,15 @@ When updating a plan: desc: \"Valid CSV imports 3 comics\" target: \"import::csv\" negative: - desc: \"Missing column errors with MissingColumn\" - target: \"import::csv\" + - desc: \"Missing column errors with MissingColumn\" + target: \"import::csv\" + - desc: \"Malformed row errors with ParseError\" + target: \"import::csv\" boundary: - desc: \"Empty file yields empty import without error\" - target: \"import::csv\" + - desc: \"Empty file yields empty import without error\" + target: \"import::csv\" + - desc: \"File with only headers yields empty import\" + target: \"import::csv\" ``` When done, add evidence and notes: @@ -306,7 +310,7 @@ Short description for providers without native calling specs: - **plan_write**: Create or update the Plan with YAML content - Format: {\"tool\": \"plan_write\", \"args\": {\"plan\": \"plan_id: my-plan\\nitems: [...]\"}} - - Example: {\"tool\": \"plan_write\", \"args\": {\"plan\": \"plan_id: feature-x\\nitems:\\n - id: I1\\n description: Add feature\\n state: todo\\n touches: [src/lib.rs]\\n checks:\\n happy: {desc: Works, target: lib}\\n negative: {desc: Errors, target: lib}\\n boundary: {desc: Edge, target: lib}\"}} + - Example: {\"tool\": \"plan_write\", \"args\": {\"plan\": \"plan_id: feature-x\\nitems:\\n - id: I1\\n description: Add feature\\n state: todo\\n touches: [src/lib.rs]\\n checks:\\n happy: {desc: Works, target: lib}\\n negative:\\n - {desc: Errors, target: lib}\\n boundary:\\n - {desc: Edge, target: lib}\"}} - **plan_approve**: Approve the current plan revision (called by user) - Format: {\"tool\": \"plan_approve\", \"args\": {}} diff --git a/crates/g3-core/src/tool_definitions.rs b/crates/g3-core/src/tool_definitions.rs index be581ba..d032361 100644 --- a/crates/g3-core/src/tool_definitions.rs +++ b/crates/g3-core/src/tool_definitions.rs @@ -289,14 +289,14 @@ fn create_core_tools(exclude_research: bool) -> Vec { - touches: Array of paths/modules affected - checks: happy: {desc, target} - Normal successful operation - negative: {desc, target} - Error handling, invalid input - boundary: {desc, target} - Edge cases, limits + negative: [{desc, target}, ...] - Error handling, invalid input (>=1 required) + boundary: [{desc, target}, ...] - Edge cases, limits (>=1 required) - evidence: Array of file:line refs, test names (required when done) - notes: Implementation explanation (required when done) Rules: - Keep items ≤ 7 by default -- All three checks (happy, negative, boundary) are required +- All checks required: 1 happy, 1+ negative, 1+ boundary - Cannot remove items from an approved plan (mark as blocked instead) - Evidence and notes required when marking item as done"#.to_string(), input_schema: json!({ diff --git a/crates/g3-core/src/tools/plan.rs b/crates/g3-core/src/tools/plan.rs index 4d77b1b..75cc73d 100644 --- a/crates/g3-core/src/tools/plan.rs +++ b/crates/g3-core/src/tools/plan.rs @@ -96,18 +96,30 @@ impl Check { pub struct Checks { /// Happy path check - normal successful operation pub happy: Check, - /// Negative case check - error handling, invalid input - pub negative: Check, - /// Boundary condition check - edge cases, limits - pub boundary: Check, + /// Negative case checks - error handling, invalid input (at least 1 required) + pub negative: Vec, + /// Boundary condition checks - edge cases, limits (at least 1 required) + pub boundary: Vec, } impl Checks { - /// Validate all three checks. + /// Validate all checks (1 happy, 1+ negative, 1+ boundary). pub fn validate(&self) -> Result<()> { self.happy.validate().map_err(|e| anyhow!("happy check: {}", e))?; - self.negative.validate().map_err(|e| anyhow!("negative check: {}", e))?; - self.boundary.validate().map_err(|e| anyhow!("boundary check: {}", e))?; + + if self.negative.is_empty() { + return Err(anyhow!("at least one negative check is required")); + } + for (i, check) in self.negative.iter().enumerate() { + check.validate().map_err(|e| anyhow!("negative check [{}]: {}", i, e))?; + } + + if self.boundary.is_empty() { + return Err(anyhow!("at least one boundary check is required")); + } + for (i, check) in self.boundary.iter().enumerate() { + check.validate().map_err(|e| anyhow!("boundary check [{}]: {}", i, e))?; + } Ok(()) } } @@ -927,8 +939,8 @@ mod tests { fn make_test_checks() -> Checks { Checks { happy: make_test_check(), - negative: make_test_check(), - boundary: make_test_check(), + negative: vec![make_test_check()], + boundary: vec![make_test_check()], } } @@ -969,6 +981,55 @@ mod tests { assert!(empty_target.validate().is_err()); } + #[test] + fn test_checks_validation_multiple_negative_and_boundary() { + // Multiple negative and boundary checks should validate + let checks = Checks { + happy: make_test_check(), + negative: vec![ + Check::new("Invalid input", "parse::input"), + Check::new("Missing file", "io::read"), + Check::new("Network error", "net::connect"), + ], + boundary: vec![ + Check::new("Empty input", "parse::input"), + Check::new("Max size input", "parse::input"), + ], + }; + assert!(checks.validate().is_ok()); + } + + #[test] + fn test_checks_validation_empty_negative_fails() { + let checks = Checks { + happy: make_test_check(), + negative: vec![], // Empty - should fail + boundary: vec![make_test_check()], + }; + let err = checks.validate().unwrap_err(); + assert!(err.to_string().contains("at least one negative check")); + } + + #[test] + fn test_checks_validation_empty_boundary_fails() { + let checks = Checks { + happy: make_test_check(), + negative: vec![make_test_check()], + boundary: vec![], // Empty - should fail + }; + let err = checks.validate().unwrap_err(); + assert!(err.to_string().contains("at least one boundary check")); + } + + #[test] + fn test_checks_validation_single_of_each_passes() { + // Minimum case: exactly 1 of each + let checks = make_test_checks(); + assert!(checks.validate().is_ok()); + assert_eq!(checks.negative.len(), 1); + assert_eq!(checks.boundary.len(), 1); + } + #[test] fn test_plan_item_validation() { let item = make_test_item("I1");