Reduce Content validator complexity for release Sonar gate

[Brad-Edwards]

Summary

PR #1026 (dev to main) is blocked by SonarCloud because Content.validate_type_requirements() has cognitive complexity 15, above the authorized threshold of 10.

Required outcome

Refactor the validator without changing Content validation behavior, add focused regression coverage where needed, and restore the Sonar quality gate for the next dev analysis.

Acceptance criteria

• Content.validate_type_requirements() is at or below the configured Sonar complexity threshold.
• Existing Content validation behavior remains covered and unchanged.
• Repository completion, policy, CI, and Sonar gates pass.

Requirements

• DSL-436 — Initial service state and native materialization semantics

[Brad-Edwards]

🛠️ Picked up by /implement - driver codex, branch 1027-reduce-content-validator-complexity, 2026-07-31T17:50:05.515Z.

[Brad-Edwards]

gc workflow phase recorded: preflight (issue #1027). Posted by the MCP server to enforce ordering between workflow steps (issue #794 MVP-2). Do not edit or delete — used by downstream tools to gate phase prerequisites.

[Brad-Edwards]

gc workflow phase recorded: plan (issue #1027). Posted by the MCP server to enforce ordering between workflow steps (issue #794 MVP-2). Do not edit or delete — used by downstream tools to gate phase prerequisites.

Implementation plan

1. Lock the existing behavior before refactoring. Treat PR Dev #1026's open Sonar python:FunctionComplexity finding as the failing structural gate. Add a compact direct-model regression matrix in implementations/python/tests/test_sdl_models.py for the uncovered search-index cases: non-dataset content, schema-only datasets carrying source, schema-only datasets carrying items, and first-error precedence. Preserve the current Pydantic ValidationError messages and ordering exactly.
2. Decompose the owning validator without changing its contract. In implementations/python/packages/raes/content.py, keep validate_type_requirements() as the single mode="after" coordinator and extract small private, validation-only helpers for the typed search-index binding and dataset payload rule. Keep ordinary file/dataset/directory checks, source is not None semantics, default-profile normalization, model output, and generated schemas unchanged. Do not add suppressions or change Sonar configuration.
3. Verify the narrow behavior and unchanged contract surface. Run the focused content-model and initial-service-state tests, confirm the generated schema bundle remains identical, and verify the refactored coordinator's branching is below the Sonar threshold through the repository's existing Sonar gate. Documentation remains unchanged because the public API, schema, behavior, and diagnostics do not change.
4. Run the required repository gates on the final tree. Run implementations/python/.venv/bin/python tools/check_repo_policy.py, the requirement-governance path under the workflow's explicit requirement-free --skip-requirement handling, implementations/python/.venv/bin/python tools/verify_all.py --skip-requirement, and the configured completion/policy/pre-commit gates. After publication, require CI and Sonar to pass before marking the PR ready.

Whole-repository design checks

• Security: retain SDLModel(extra="forbid"), Pydantic's validation envelope, and constant bounded error messages; helpers will not log, mutate, normalize, reconstruct models, or expose target/source/item values. No auth, secret, environment, filesystem, network, subprocess, or argv surface changes.
• Maintainability: reuse the existing Pydantic model validator and the repository's thin-validator/private-helper pattern. Each helper remains independently below the configured complexity threshold; no registry, new DTO, exception hierarchy, or duplicated validation framework is introduced.
• Extensibility: narrow profile-specific validation on the existing discriminated ServiceMaterializationProfile type, so a future materialization profile can add one bounded helper without reopening ordinary content-type rules.
• Whole-repo view: the parser/error adapter, instantiation revalidation, semantic validator, compiler, canonical digesting, published schemas, and backend admission continue consuming the same Content model output. Existing test, schema-parity, policy, Nox, CI, and Sonar gates verify that those downstream surfaces remain unchanged.

[Brad-Edwards]

gc workflow phase recorded: preflight (issue #1027). Posted by the MCP server to enforce ordering between workflow steps (issue #794 MVP-2). Do not edit or delete — used by downstream tools to gate phase prerequisites.

[Brad-Edwards]

gc workflow phase recorded: plan (issue #1027). Posted by the MCP server to enforce ordering between workflow steps (issue #794 MVP-2). Do not edit or delete — used by downstream tools to gate phase prerequisites.

Implementation plan amendment — DSL-436 binding

The repository governance gate confirmed that the changed Content surface belongs to the existing active requirement DSL-436. Replace the requirement-free verification wording in the earlier plan with the following binding:

• Run all repository-authored gates with RAES_REQUIREMENT_UID=DSL-436, including:
  • implementations/python/.venv/bin/python tools/check_repo_policy.py;
  • implementations/python/.venv/bin/python tools/check_requirement_governance.py;
  • implementations/python/.venv/bin/python tools/verify_all.py; and
  • the configured completion, policy, pre-commit, CI, and Sonar gates.
• Preserve every DSL-436 clause rather than changing its semantics: the versioned content identity, exact service-target materialization requirements, reset ownership, deterministic generation inputs, participant-equivalent readback, provider neutrality, and prohibition on a separate narrative-history object class all continue through the same typed Content output and downstream compiler/runtime boundaries.
• Add traceability for the touched content.py validator and focused model regression test during post-merge reconciliation; retain all existing DSL-436 implementation, specification, and test links.
• The implementation remains the two-file, behavior-preserving decomposition already described. No schema, specification, ADR, compiler, planner, runtime, backend, changelog, version, or Sonar-configuration change is authorized.

[Brad-Edwards]

gc_codex_review — cycle 1 of 1 (pre-push) on issue #1027 (branch 1027-reduce-content-validator-complexity)
Diff mode: inline — the complete diff was supplied in one prompt

Core review

Verdict: ship

This is shaped correctly. The change isolates the search-index-schema exception from ordinary dataset validation while preserving the existing model-validation boundary and behavior. The tests pin the materialization’s key exclusion rules and validation precedence without introducing fixture-aware production logic or a speculative abstraction. The seam remains local to Content and leaves room for additional materialization-specific rules.

No blocking findings.

Security review

Verdict: ship

This is shaped correctly: the change extracts existing content-shape validation into narrowly scoped helpers while preserving the validator’s security-relevant constraints and validation order. The tests cover incompatible content types, prohibited source/items payloads, and the required target anchor. It introduces no new trust boundary, input sink, authorization path, secret handling, or unsafe parsing behavior, and it does not foreclose the obvious next materialization-specific validation variation.

No blocking findings.

[Brad-Edwards]

gc_codex_review pre-push cycle 1 of 1 complete for issue #1027 on branch '1027-reduce-content-validator-complexity'. Posted by the MCP server to enforce the pre-push hard-cap-1 contract (issues #796, #804, #906). Do not edit or delete — used by the next gc_codex_review (uncommitted) invocation to count cycles.

[Brad-Edwards]

Review decision record — codex cycle 1 (issue #1027)

Reviewer: codex
Cycle: 1

Architectural read:

Core reviewer: This is shaped correctly. The change isolates the search-index-schema exception from ordinary dataset validation while preserving the existing model-validation boundary and behavior. The tests pin the materialization’s key exclusion rules and validation precedence without introducing fixture-aware production logic or a speculative abstraction. The seam remains local to Content and leaves room for additional materialization-specific rules.

Security reviewer: This is shaped correctly: the change extracts existing content-shape validation into narrowly scoped helpers while preserving the validator’s security-relevant constraints and validation order. The tests cover incompatible content types, prohibited source/items payloads, and the required target anchor. It introduces no new trust boundary, input sink, authorization path, secret handling, or unsafe parsing behavior, and it does not foreclose the obvious next materialization-specific validation variation.

Blocking findings: 0 (clean run)

[Brad-Edwards]

gc_test_quality_review cycle 1 of 1 — issue #1027

Reviewer: test-quality (claude-sonnet-5 via gc_test_quality_review)
Branch: 1027-reduce-content-validator-complexity
Cycle: 1 / 1
Findings: 1

Finding 1 — [critical] implementations/python/tests/test_sdl_models.py::TestContent::test_search_index_schema_content_shape_rules

Problem: All four parametrize cases for the refactored Content validator wrap pytest.raises(ValidationError, ...); none constructs a valid ServiceSearchIndexSchemaMaterialization dataset content (proper field_semantics, no source/items) and asserts it succeeds. The success branch of both new helpers -- _validate_search_index_schema_content's final return True and _validate_ordinary_dataset_content's is_search_index_schema skip -- is therefore never reached by this test.
Why it matters: If the refactor's boolean plumbing regresses (e.g. is_search_index_schema is dropped, hardcoded False, or the skip condition is inverted), every one of the four existing cases still passes -- cases 1-3 fail before the plumbing even matters (wrong content type / source-or-items present), and case 4 fails on the earlier target check. A regression that makes legitimate search-index-schema-only dataset content wrongly raise 'Dataset content requires either 'source' or non-empty 'items'' would go undetected by this test file, which is the direct unit-level guard for Content's validator.
Fix: Add a fifth parametrize entry (or a standalone test) that calls Content.model_validate with type='dataset', a target, a valid service_materialization built from _search_index_schema_materialization(), and no source/items, and asserts it validates without raising -- e.g. assert isinstance(content.service_materialization, ServiceSearchIndexSchemaMaterialization) and content.items == [] and content.source is None.

[Brad-Edwards]

gc_test_quality_review cycle 1 of 1 complete for issue #1027 on branch '1027-reduce-content-validator-complexity'. Posted by the MCP server to enforce the gc_test_quality_review hard-cap-1 contract (issue #884 follow-up, default lowered in #906). Do not edit or delete — used by the next gc_test_quality_review invocation to count cycles.

[Brad-Edwards]

Review decision record — test-quality cycle 1 (issue #1027)

Reviewer: test-quality
Cycle: 1

Architectural read:

This is a straightforward complexity-reduction refactor: Content.validate_type_requirements was split into two guard-clause helpers, _validate_search_index_schema_content and _validate_ordinary_dataset_content, composed in the exact order the monolithic validator used, preserving every existing error message and evaluation sequence (target -> path -> search-index-schema shape -> ordinary dataset shape -> directory destination). That's the right shape for a Sonar-complexity remediation: mechanical extraction, no new abstraction, no behavior change. The accompanying test, test_search_index_schema_content_shape_rules, is clearly written as a characterization test to lock the refactor down. Its one gap is that it exercises exclusively the raise branches of the two new helpers and never exercises the branch the refactor exists to support: a well-formed ServiceSearchIndexSchemaMaterialization dataset content that succeeds.

Blocking findings: 1

Finding 1 — one-off

• ID: F1
• Title: (no title)
• Decision: fix
• Rationale: Addressed by next cycle

[Brad-Edwards]

Resolved by #1029.