[jonahgabriel]

Summary

• Resolves validator-policy mismatch between validate-yaml (accepts string schema_version) and validate-string-versions (rejects it)
• Chose Option 2: ticket contracts are governance artifacts with string-shaped schema_version, enforced by ModelTicketContract.field_validator
• Extended validate_string_versions.py to cover YAML files with explicit contracts/OMN-*.yaml exemption
• Updated pre-commit hook from types: [python] to types_or: [python, yaml]
• 9 new tests proving Python detection, YAML detection, and ticket-contract exemption
• 1161 tests pass, 0 failures

Summary by CodeRabbit

Release Notes

• New Features

  • Version validation now extends to YAML configuration files, with targeted exemptions for specific contract files.

• Tests

  • Added comprehensive unit test coverage for version validation logic across Python and YAML formats.

[coderabbitai]

Warning

Rate limit exceeded

@jonahgabriel has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 2 minutes and 33 seconds before requesting another review.

To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 63a519e3-8f64-45a3-a65f-a382b9b3f8d6

📥 Commits

Reviewing files that changed from the base of the PR and between 060b7a4 and ebeb50b.

📒 Files selected for processing (3)

• .pre-commit-config.yaml
• scripts/validation/validate_string_versions.py
• tests/unit/test_validate_string_versions.py

📝 Walkthrough

Walkthrough

The PR extends a pre-commit validation hook to detect hardcoded version strings across both Python and YAML files. The validation script now checks Python __init__.py files for direct version assignments and scans YAML configuration files for semver-like string values, with exemptions for specific ticket-contract files. Comprehensive unit tests are added.

Changes

Cohort / File(s)	Summary
Pre-commit Hook Configuration .pre-commit-config.yaml	Updated hook name and filtering to reference "Python + YAML" anti-patterns; changed from types: [python] to types_or: [python, yaml] to broaden scope.
Validation Script Enhancement scripts/validation/validate_string_versions.py	Extended to validate both Python __init__.py files for hardcoded __version__ assignments and YAML files for semver-like string versions in version-related fields. Added semver pattern matching and exemption logic for contracts/OMN-*.yaml files.
Unit Test Coverage tests/unit/test_validate_string_versions.py	New test module validating helper function behavior: hardcoded version detection in Python, string version detection in YAML, non-hardcoded patterns (e.g., importlib.metadata), mapping-based versions, and ticket-contract exemptions.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 A hop, skip, and version check away!
Strings in YAML, now caught each day,
Python __init__ won't pass our sight,
With semver patterns shining bright,
Contracts exempted, the tests align,
Clean versions dance—oh, how they shine! 🌟

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 26.67% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: formalizing a string schema_version exemption for ticket contracts in the validation script.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch jonah/omn-9593-align-schema-version-policy

---

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

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (1)

tests/unit/test_validate_string_versions.py (1)

58-100: Add regression tests for quote-style and path-boundary edge cases.

Please add tests for (1) '1.2.3' single-quoted YAML versions and (2) a non-exempt path like mycontracts/OMN-9593.yaml to lock in the intended policy boundaries.

Suggested test additions

 class TestYamlStringVersion:
+    def test_detects_single_quoted_version_in_yaml(self, tmp_path: Path) -> None:
+        p = _write(tmp_path, "config.yaml", "version: '1.0.0'\n")
+        assert len(_has_string_version_in_yaml(p)) == 1
+
 class TestTicketContractExemption:
+    def test_mycontracts_path_not_exempt(self, tmp_path: Path) -> None:
+        p = _write(
+            tmp_path,
+            "mycontracts/OMN-9593.yaml",
+            'schema_version: "1.0.0"\n',
+        )
+        assert _is_ticket_contract(p) is False
+        assert len(_has_string_version_in_yaml(p)) == 1

🤖 Prompt for AI Agents

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

In `@tests/unit/test_validate_string_versions.py` around lines 58 - 100, Add two
new unit tests: one in TestYamlStringVersion that writes a YAML file containing
a single-quoted version string like 'version: \'1.2.3\'' and asserts
_has_string_version_in_yaml(path) returns a violation (len==1 and contains
"ModelSemVer"); and one in TestTicketContractExemption that writes a file at
path "mycontracts/OMN-9593.yaml" with schema_version: "1.0.0" and asserts
_is_ticket_contract(path) is False and _has_string_version_in_yaml(path) reports
the string-version violation; use the existing helpers (_write,
_has_string_version_in_yaml, _is_ticket_contract) and mirror naming/structure of
the surrounding tests.

🤖 Prompt for all review comments with AI agents

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

Inline comments:
In `@scripts/validation/validate_string_versions.py`:
- Around line 34-38: The current TICKET_CONTRACT_GLOB and _is_ticket_contract
allow false positives and fail on Windows paths; update TICKET_CONTRACT_GLOB to
be anchored (e.g. r"^contracts/OMN-\d+\.yaml$") and change _is_ticket_contract
to operate on Path.as_posix() and use fullmatch/match instead of search, so it
only matches a path that exactly equals "contracts/OMN-<digits>.yaml" across
platforms; reference TICKET_CONTRACT_GLOB and _is_ticket_contract when making
these changes.
- Line 30: The _SEMVER_RE currently only matches unquoted or double-quoted
versions; update the regex _SEMVER_RE to accept optional single or double quotes
around the SemVer (e.g., allow leading and trailing ' or "), then update any
usage that relies on this pattern (the validation at the usage site referenced
around line 114) so it still strips or validates the captured groups correctly
(ensure the groups capture the numeric major.minor.patch regardless of quoting).
Use the symbol _SEMVER_RE to locate the change and adjust the validation logic
where it's applied.

---

Nitpick comments:
In `@tests/unit/test_validate_string_versions.py`:
- Around line 58-100: Add two new unit tests: one in TestYamlStringVersion that
writes a YAML file containing a single-quoted version string like 'version:
\'1.2.3\'' and asserts _has_string_version_in_yaml(path) returns a violation
(len==1 and contains "ModelSemVer"); and one in TestTicketContractExemption that
writes a file at path "mycontracts/OMN-9593.yaml" with schema_version: "1.0.0"
and asserts _is_ticket_contract(path) is False and
_has_string_version_in_yaml(path) reports the string-version violation; use the
existing helpers (_write, _has_string_version_in_yaml, _is_ticket_contract) and
mirror naming/structure of the surrounding tests.

🪄 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: 434edb9a-45ce-4770-bc05-3e06cc8a20db

📥 Commits

Reviewing files that changed from the base of the PR and between acbea5f and 060b7a4.

📒 Files selected for processing (3)

• .pre-commit-config.yaml
• scripts/validation/validate_string_versions.py
• tests/unit/test_validate_string_versions.py