Skip to content

refactor: decompose ultra receipt validator semantics#14

Open
hummbl-dev wants to merge 1 commit into
mainfrom
refactor/ultra-receipt-validator-decomposition
Open

refactor: decompose ultra receipt validator semantics#14
hummbl-dev wants to merge 1 commit into
mainfrom
refactor/ultra-receipt-validator-decomposition

Conversation

@hummbl-dev

Copy link
Copy Markdown
Owner

Summary

  • Decomposed _validate_schema_subset (cyclomatic complexity 58) into 8 focused helpers
  • Decomposed _validate_receipt_semantics (cyclomatic complexity 58) into 7 domain helpers
  • Added 24 direct helper unit tests

Closes #11

Changes

Schema-keyword validation split

  • _validate_ref — $ref resolution
  • _validate_oneof — oneOf branch matching
  • _validate_type — type keyword
  • _validate_const_enum — const and enum keywords
  • _validate_object_keywords — required, properties, additionalProperties
  • _validate_array_keywords — minItems, maxItems, uniqueItems, items
  • _validate_string_keywords — minLength, pattern, format
  • _validate_number_keywords — finite, minimum, maximum

Receipt cross-field rules split by domain

  • _validate_schema_identity — schema_id and schema_version
  • _validate_model_semantics — model.orchestration_agent_count
  • _validate_telemetry_semantics — telemetry metric units, finiteness, integer constraints
  • _validate_assessment_semantics — duplicate_work and synthesis_loss
  • _validate_delegation_lanes_semantics — lane uniqueness, state, timing
  • _validate_outcome_semantics — outcome_improvement
  • _validate_routing_semantics — routing_decision consistency

Acceptance criteria

  • Split schema-keyword validation into focused helpers without changing supported Draft 2020-12 behavior
  • Split receipt cross-field rules by domain: model/delegation, telemetry, duplication/synthesis, timing, outcome, and routing
  • Preserve deterministic, sorted JSON-path error messages
  • Preserve all valid/invalid fixture outcomes and add direct helper tests
  • Raise receipt-validator branch coverage to at least 80% — 85% achieved
  • Keep runtime dependencies at zero
  • Run Ruff, mypy, Bandit, and the complete local test suite — all clean
  • Do not use GitHub Actions while organizational Actions minutes remain exhausted

Scope boundary

No v0.1 schema contract or route policy changes. Mechanical decomposition only.

Validation

  • 99 tests + 96 subtests pass
  • Ruff: clean
  • mypy: clean
  • Bandit: zero findings
  • Branch coverage: 85%

Closes #11

Split _validate_schema_subset (cyclomatic complexity 58) into 8 focused
helpers:
- _validate_ref
- _validate_oneof
- _validate_type
- _validate_const_enum
- _validate_object_keywords
- _validate_array_keywords
- _validate_string_keywords
- _validate_number_keywords

Split _validate_receipt_semantics (cyclomatic complexity 58) into 7
domain helpers:
- _validate_schema_identity
- _validate_model_semantics
- _validate_telemetry_semantics
- _validate_assessment_semantics
- _validate_delegation_lanes_semantics
- _validate_outcome_semantics
- _validate_routing_semantics

Added 24 direct helper unit tests (HelperUnitTests class) covering
edge cases, error paths, and type guards.

Results:
- All 99 tests + 96 subtests pass
- Branch coverage on validator: 85% (exceeds 80% requirement)
- Ruff: clean
- mypy: clean
- Bandit: zero findings
- Zero runtime dependencies preserved
- Deterministic sorted JSON-path error messages preserved
- All valid/invalid fixture outcomes preserved
- No v0.1 schema contract changes
@coderabbitai

coderabbitai Bot commented Jul 11, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@hummbl-dev, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 3 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: d775e587-354f-4385-8787-1e9e50441c29

📥 Commits

Reviewing files that changed from the base of the PR and between 4e9580f and a420500.

📒 Files selected for processing (2)
  • tests/test_ultra_evaluation_receipt.py
  • tools/validate_ultra_evaluation_receipt.py
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch refactor/ultra-receipt-validator-decomposition

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@hummbl-dev

Copy link
Copy Markdown
Owner Author

Peer Review

This PR decomposes the monolithic _validate_schema_subset and _validate_receipt_semantics functions into focused helper functions with unit tests. The receipt-semantics decomposition is faithful — all type guards, error messages, and check ordering are preserved. The schema-subset decomposition has one behavioral change worth noting.

Verdict: APPROVE

Findings

  1. (Low) Type-validation early-return semantics changed. In the old _validate_schema_subset, a failed type check issued return from the function itself, skipping const/enum and all type-specific keyword checks. In the new code, _validate_type returns from itself, but _validate_schema_subset continues to call _validate_const_enum and the remaining keyword validators. For a schema node with both type and const/enum (e.g., {"type": "string", "const": "foo"}) where the value has the wrong type, the new code will emit both "expected string" and "must equal "foo"" errors instead of just the former. This does not change pass/fail outcomes (the receipt is still invalid either way), but it does change error output for already-invalid inputs. If any downstream consumers assert on exact error sets, this could surface as a diff. Consider adding an early-return in _validate_schema_subset after _validate_type if type errors are present, to preserve the original behavior exactly.

  2. (Info) model fetched twice across decomposed functions. _validate_model_semantics and _validate_routing_semantics each call receipt.get("model") independently, whereas the old code fetched it once. This is semantically identical (dict.get is idempotent) and not a concern — just noting the micro-change.

  3. (Info) Good test coverage for the decomposition. The new HelperUnitTests class directly exercises the extracted helpers, including edge cases like non-dict/non-list early returns, bool exclusion from number keywords, leap-second validation, and $ref resolution failures. This is solid for a refactor.

@hummbl-dev
hummbl-dev marked this pull request as ready for review July 13, 2026 07:36
@hummbl-dev

hummbl-dev commented Jul 13, 2026

Copy link
Copy Markdown
Owner Author

Initial review pass — pinned head a420500ed39f against main.

Evidence:

  • Scope: 2 files, +489/-228. Notable paths: tests/test_ultra_evaluation_receipt.py, tools/validate_ultra_evaluation_receipt.py
  • Mergeability: MERGEABLE
  • Head checks: PASS — 2 reported checks have no failing or pending result

Findings:

  • P3: 31 added normative MUST/SHALL/REQUIRED line(s) require authority-source validation.

Next review focus: Check routing defaults, fallback/fail-closed behavior, schema compatibility, and negative-path tests before treating the policy as executable.

Gate: CONTINUE. This is an evidence-backed triage comment, not final merge approval; promotion still requires content validation against this exact head and the repository's review rules.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

refactor: decompose Ultra receipt validator semantics

1 participant