Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions specs/001-agentready-scorer/contracts/assessment-schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -74,11 +74,13 @@
},
"attributes_total": {
"type": "integer",
"const": 25
"minimum": 10,
"maximum": 25,
"description": "Total attributes in assessment (may be < 25 if --exclude was used)"
},
"findings": {
"type": "array",
"minItems": 25,
"minItems": 10,
"maxItems": 25,
"items": {
"type": "object",
Expand Down
47 changes: 47 additions & 0 deletions src/agentready/services/schema_validator.py
Original file line number Diff line number Diff line change
Expand Up @@ -139,8 +139,55 @@ def additional_properties(validator, aP, instance, schema):
except Exception as e:
errors.append(f"Validation error: {str(e)}")

# Cross-field validation: JSON Schema draft-07 cannot enforce these constraints
# Fix for #309: ensure cross-field consistency after relaxing schema constraints
cross_field_errors = self._validate_cross_field_constraints(report_data)
errors.extend(cross_field_errors)

return (len(errors) == 0, errors)

def _validate_cross_field_constraints(
self, report_data: dict[str, Any]
) -> list[str]:
"""Validate cross-field constraints that JSON Schema cannot express.

These constraints are documented in specs/001-agentready-scorer/data-model.md
but cannot be enforced by JSON Schema draft-07.

Args:
report_data: Parsed JSON report data

Returns:
List of validation error messages (empty if valid)
"""
errors = []

# Constraint 1: len(findings) == attributes_total
findings = report_data.get("findings", [])
attributes_total = report_data.get("attributes_total", 0)

if len(findings) != attributes_total:
errors.append(
f"findings count ({len(findings)}) must equal "
f"attributes_total ({attributes_total})"
)

# Constraint 2: attributes_assessed + attributes_skipped == attributes_total
# Note: attributes_skipped is the canonical key, attributes_not_assessed is deprecated
attributes_assessed = report_data.get("attributes_assessed", 0)
attributes_skipped = report_data.get(
"attributes_skipped", report_data.get("attributes_not_assessed", 0)
)

if attributes_assessed + attributes_skipped != attributes_total:
errors.append(
f"attributes_assessed ({attributes_assessed}) + "
f"attributes_skipped ({attributes_skipped}) must equal "
f"attributes_total ({attributes_total})"
)

return errors

def validate_report_file(
self, report_path: Path, strict: bool = True
) -> tuple[bool, list[str]]:
Expand Down
149 changes: 149 additions & 0 deletions tests/unit/test_schema_validator.py
Original file line number Diff line number Diff line change
Expand Up @@ -165,3 +165,152 @@ def test_validate_lenient_mode(validator, valid_report_data):

# Lenient mode should pass
assert is_valid is True or len(errors) == 0


def test_validate_excluded_attributes_issue_309(validator, valid_report_data):
"""Test validation passes for assessments with excluded attributes.

Regression test for issue #309: Schema rejected valid assessments
generated with --exclude flags because it hardcoded attributes_total=25.

The schema now allows 10-25 attributes to support exclusions.
"""
# Simulate an assessment with 15 excluded attributes (10 remaining)
valid_report_data["attributes_total"] = 10
valid_report_data["attributes_assessed"] = 9
valid_report_data["attributes_skipped"] = 1
valid_report_data["findings"] = valid_report_data["findings"][:10]

is_valid, errors = validator.validate_report(valid_report_data)

assert is_valid is True, f"Validation failed unexpectedly: {errors}"
assert len(errors) == 0


def test_validate_partial_exclusion_issue_309(validator, valid_report_data):
"""Test validation passes for assessments with some excluded attributes.

Tests the common case from issue #309: user excludes 3 attributes,
resulting in 22 attributes instead of 25.
"""
# Simulate PR #301: 22 attributes (3 excluded)
valid_report_data["attributes_total"] = 22
valid_report_data["attributes_assessed"] = 21
valid_report_data["attributes_skipped"] = 1
valid_report_data["findings"] = valid_report_data["findings"][:22]

is_valid, errors = validator.validate_report(valid_report_data)

assert is_valid is True, f"Validation failed unexpectedly: {errors}"
assert len(errors) == 0


def test_validate_too_few_attributes_rejected(validator, valid_report_data):
"""Test validation fails when too many attributes are excluded.

The schema requires at least 10 attributes to ensure meaningful assessments.
"""
# Try to submit with only 5 attributes (below minimum of 10)
valid_report_data["attributes_total"] = 5
valid_report_data["attributes_assessed"] = 5
valid_report_data["attributes_skipped"] = 0
valid_report_data["findings"] = valid_report_data["findings"][:5]

is_valid, errors = validator.validate_report(valid_report_data)

assert is_valid is False, "Should reject assessments with fewer than 10 attributes"
assert len(errors) > 0


# Cross-field validation tests (PR #312 review comment)
# These test the _validate_cross_field_constraints() method


def test_cross_field_findings_count_mismatch(validator, valid_report_data):
"""Test validation fails when findings count doesn't match attributes_total.

Regression test for PR #312 review: JSON Schema cannot enforce that
len(findings) == attributes_total, so we need programmatic validation.
"""
# Set attributes_total to 10 but keep 25 findings
valid_report_data["attributes_total"] = 10
valid_report_data["attributes_assessed"] = 9
valid_report_data["attributes_skipped"] = 1
# Intentionally leave findings at 25 items (mismatch)

is_valid, errors = validator.validate_report(valid_report_data)

assert is_valid is False, "Should reject when findings count != attributes_total"
assert any("findings count" in err and "must equal" in err for err in errors)


def test_cross_field_assessed_skipped_sum_mismatch(validator, valid_report_data):
"""Test validation fails when assessed + skipped != attributes_total.

Regression test for PR #312 review: JSON Schema cannot enforce that
attributes_assessed + attributes_skipped == attributes_total.
"""
# Set values that don't add up: 20 + 5 = 25, but total is 10
valid_report_data["attributes_total"] = 10
valid_report_data["attributes_assessed"] = 20
valid_report_data["attributes_skipped"] = 5
valid_report_data["findings"] = valid_report_data["findings"][:10]

is_valid, errors = validator.validate_report(valid_report_data)

assert is_valid is False, "Should reject when assessed + skipped != total"
assert any("attributes_assessed" in err and "must equal" in err for err in errors)


def test_cross_field_both_constraints_violated(validator, valid_report_data):
"""Test validation catches multiple cross-field violations.

Both constraints should be checked and all errors reported.
"""
# Violate both constraints
valid_report_data["attributes_total"] = 10
valid_report_data["attributes_assessed"] = 20
valid_report_data["attributes_skipped"] = 5
# Keep 25 findings (mismatch with attributes_total=10)

is_valid, errors = validator.validate_report(valid_report_data)

assert is_valid is False
assert len(errors) >= 2, "Should report both cross-field validation errors"
assert any("findings count" in err for err in errors)
assert any("attributes_assessed" in err for err in errors)


def test_cross_field_valid_partial_assessment(validator, valid_report_data):
"""Test validation passes when all cross-field constraints are satisfied.

A valid partial assessment should pass all checks.
"""
# Valid partial assessment: 15 attributes excluded, 10 remaining
valid_report_data["attributes_total"] = 10
valid_report_data["attributes_assessed"] = 8
valid_report_data["attributes_skipped"] = 2
valid_report_data["findings"] = valid_report_data["findings"][:10]

is_valid, errors = validator.validate_report(valid_report_data)

assert is_valid is True, f"Valid partial assessment should pass: {errors}"
assert len(errors) == 0


def test_cross_field_deprecated_attributes_not_assessed(validator, valid_report_data):
"""Test validation works with deprecated 'attributes_not_assessed' key.

The deprecated key should be supported for backwards compatibility.
"""
# Use deprecated key name
valid_report_data["attributes_total"] = 10
valid_report_data["attributes_assessed"] = 8
del valid_report_data["attributes_skipped"]
valid_report_data["attributes_not_assessed"] = 2
valid_report_data["findings"] = valid_report_data["findings"][:10]

is_valid, errors = validator.validate_report(valid_report_data)

assert is_valid is True, f"Should accept deprecated key: {errors}"
assert len(errors) == 0
Loading