[jonahgabriel]

Summary

• Adds integration tests for the handler-contract compliance scanner
• Validates cross_reference scanner against synthetic node directories with known compliance states
• Covers MISSING_HANDLER_ROUTING, HARDCODED_TOPIC, LOGIC_IN_NODE violations and verdict thresholds (COMPLIANT/HYBRID/IMPERATIVE/ALLOWLISTED)

Test plan

• All tests pass locally with pytest tests/test_handler_contract_compliance_scan.py
• CI passes

Summary by CodeRabbit

• Tests
  • Added a comprehensive integration test suite for the contract compliance scanner, covering verdicts (compliant, hybrid, imperative, missing contract, allowlisted), violation detection, and edge cases (missing or empty handler layouts and node logic detection).
  • Added a unit-test helper to ensure test-created git repositories are properly configured before commits/tags.

[coderabbitai]

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

Adds a new integration test module validating onex_change_control.scanners.handler_contract_compliance.cross_reference across multiple verdicts and edge cases, and introduces a git repo initialization helper used by unit tests to ensure git user config before commits/tags.

Changes

Cohort / File(s)	Summary
Handler Compliance Scanner Tests tests/test_handler_contract_compliance_scan.py	Adds an extensive pytest integration suite that creates synthetic src/.../nodes/... layouts, writes contract.yaml, handlers/<handler>.py, and node.py, then asserts cross_reference(...) returns expected verdicts (COMPLIANT, HYBRID, IMPERATIVE, MISSING_CONTRACT, ALLOWLISTED) and specific violation details; includes edge-case tests for missing/empty handlers/ and node logic detection.
Git repo helper in unit tests tests/unit/test_sync_version_matrix.py	Adds _init_git_repo(repo: Path) helper and updates tests to use it so test-created git repos have user.email/user.name configured before committing and tagging.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐇 I dug a test burrow deep and neat,

handlers, contracts, and nodes to meet,
I stamp my paw on git with care,
Commits and verdicts hop through the air,
A tiny rabbit cheers — tests complete!

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: adding integration tests for handler-contract compliance scanning functionality.
Docstring Coverage	✅ Passed	Docstring coverage is 93.33% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch jonah/omn-6844-compliance-validation

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@tests/test_handler_contract_compliance_scan.py`:
- Around line 93-110: The test fixture declares routing for a handler identity
(e.g., HandlerExample) but the provided handler is a bare function, so the
scanner cannot match the routing entry; update the test's handler_code in
test_handler_registered_in_routing_no_violations to define a class with the
routed name that implements handle (e.g., class HandlerExample: def handle(self,
data): ...) so the scanner resolves the identity, and make the same change for
the related fixture referenced at lines 114-115.
- Around line 80-85: node_code is written verbatim which preserves caller
indentation and can produce malformed/shifted source so the scanner misses
LOGIC_IN_NODE; change the assignment of node_py_content to apply textwrap.dedent
to node_code (e.g. node_py_content = textwrap.dedent(node_code) if node_code
else textwrap.dedent("""...""")) before calling (node_dir /
"node.py").write_text(...); make the same dedent change for the other
occurrences referenced around the node_py_content creation sites so all inserted
fixtures are normalized.
- Around line 242-249: The test constructs an absolute handler_path and passes
it into allowlisted_paths, which mismatches the scanner's canonical handler_path
format; instead, call cross_reference() first (the function under test) and use
the handler_path value it returns (from the results returned by cross_reference)
as the key for allowlisted_paths/ALLOWLISTED assertions so no path-normalization
mismatch occurs—apply this change where handler_path is built (the handler_path
variable usage) and at the other occurrence around lines 251–252 in the test.

🪄 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: 20c2cf40-7a45-40ac-a711-ef835a5b057c

📥 Commits

Reviewing files that changed from the base of the PR and between 68d13c7 and 097de77.

📒 Files selected for processing (1)

• tests/test_handler_contract_compliance_scan.py