Repository navigation
feat(contracts): backfill golden_path and dod_evidence for all 7 existing contracts [OMN-7430] - #138
Conversation
…ting contracts [OMN-7430] All 7 contracts now have dod_evidence with executable checks derived from their Linear ticket requirements. 5 seam tickets have golden_path (OMN-3940, OMN-3944, OMN-3946, OMN-3947, OMN-5291). 2 non-applicable contracts have golden_path: null with comments (OMN-3943 internal reducer, OMN-5168 schema). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdded golden_path scenarios and dod_evidence entries to multiple contract YAMLs, defining end-to-end input→output event mappings, fixture sources, correlation passthrough, schema/assertion checks, timeouts, and manual DoD checklists (pytest/mypy/CLI checks). No code/API signatures changed. Changes
Sequence Diagram(s)sequenceDiagram
participant Tester as Tester (fixture/runner)
participant KafkaIn as Kafka Input Topic
participant Service as Service / Consumer
participant KafkaOut as Kafka Output Topic
participant Validator as Schema/assertion check
Tester->>KafkaIn: publish message (fixture, correlation_id)
KafkaIn->>Service: deliver event/command
Service->>Validator: validate schema & assertions
alt assertions pass
Service->>KafkaOut: emit output event (propagate correlation_id)
KafkaOut->>Tester: tester observes expected topic & payload
else assertion fail
Service->>Tester: report failure / timeout
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@contracts/OMN-3946.yaml`:
- Around line 59-64: The DoD check named dod-002 currently uses check_type:
"file_exists" which only proves the file is present; change the check (in the
checks array for dod-002) to use check_type: "command" and supply a command that
actually parses ".github/workflows/artifact-reconciliation-webhook.yml" with a
YAML parser (for example yq, yamllint, or a small python script using PyYAML) so
the check fails on syntax errors; update the check_value to that command and
keep the descriptive text "GitHub Action workflow is syntactically valid" to
match the new behavior.
- Around line 71-73: The grep check under check_value uses the wrong key: change
the dict key from file to path so the grep check is {pattern: "SASL_SSL", path:
"scripts/publish_pr_webhook_event.py"}; update the entry referenced by
check_value in the contract (the grep check against
scripts/publish_pr_webhook_event.py) to use path instead of file so it conforms
to ModelTicketContract's expected grep schema.
🪄 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: 19c1b852-d365-4f1f-b10c-59f04c375d22
📒 Files selected for processing (7)
contracts/OMN-3940.yamlcontracts/OMN-3943.yamlcontracts/OMN-3944.yamlcontracts/OMN-3946.yamlcontracts/OMN-3947.yamlcontracts/OMN-5168.yamlcontracts/OMN-5291.yaml
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
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 `@contracts/OMN-5291.yaml`:
- Around line 40-41: The golden_path.input references an input topic
"onex.cmd.onex-change-control.validate-yaml.v1" and a fixture
"tests/fixtures/validate_yaml_trigger.json" that are not present in the Kafka
flow or repo; update the contract so the topic matches an existing, executable
Kafka topic used in the flow (or add a producer/route that emits to
"onex.cmd.onex-change-control.validate-yaml.v1") and ensure the fixture file
exists at tests/fixtures/validate_yaml_trigger.json (or change the fixture path
to the correct existing fixture); verify by running the golden path test that
the topic receives the message and the fixture is loadable.
- Around line 53-68: Add a new dod_evidence entry to this YAML to run the
validate-yaml CLI against the appropriate Pydantic schema: include a check with
check_type "command" and check_value "uv run validate-yaml --schema
ModelTicketContract <path-to-this-file>" (reference the existing dod_evidence
list and the ticket contract schema ModelTicketContract) so the contract file
explicitly records schema validation alongside the pytest and mypy checks.
- Around line 46-50: The contract currently references schema_name
"ModelGovernanceCheckCompleted" and asserts field "exit_code == 0", but the
emitter emits fields like "check_type", "target", "passed", "violation_count",
and "details"; update the schema_name and assertions to match the actual payload
— replace or rename "ModelGovernanceCheckCompleted" if wrong and remove the
"exit_code" assertion, then add assertions for the real fields (e.g., assert
"passed" is boolean or true/false as expected, "violation_count" is a
non-negative integer, "check_type" and "target" are strings, and "details"
exists/is an object or array) using the exact field names from the emitter so
the contract aligns with emitted events.
🪄 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: d53d7ed3-76af-4516-9968-0bc18e2de4ce
📒 Files selected for processing (6)
contracts/OMN-3940.yamlcontracts/OMN-3943.yamlcontracts/OMN-3944.yamlcontracts/OMN-3946.yamlcontracts/OMN-5168.yamlcontracts/OMN-5291.yaml
✅ Files skipped from review due to trivial changes (2)
- contracts/OMN-5168.yaml
- contracts/OMN-3944.yaml
🚧 Files skipped from review as they are similar to previous changes (3)
- contracts/OMN-3943.yaml
- contracts/OMN-3946.yaml
- contracts/OMN-3940.yaml
| topic: "onex.cmd.onex-change-control.validate-yaml.v1" | ||
| fixture: "tests/fixtures/validate_yaml_trigger.json" |
There was a problem hiding this comment.
golden_path.input is currently non-executable
Line 40 references an input topic that is not represented in the current Kafka flow, and Line 41 references a fixture path that is missing. This makes the golden path unverifiable in practice.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@contracts/OMN-5291.yaml` around lines 40 - 41, The golden_path.input
references an input topic "onex.cmd.onex-change-control.validate-yaml.v1" and a
fixture "tests/fixtures/validate_yaml_trigger.json" that are not present in the
Kafka flow or repo; update the contract so the topic matches an existing,
executable Kafka topic used in the flow (or add a producer/route that emits to
"onex.cmd.onex-change-control.validate-yaml.v1") and ensure the fixture file
exists at tests/fixtures/validate_yaml_trigger.json (or change the fixture path
to the correct existing fixture); verify by running the golden path test that
the topic receives the message and the fixture is loadable.
| schema_name: "ModelGovernanceCheckCompleted" | ||
| assertions: | ||
| - field: "exit_code" | ||
| op: "eq" | ||
| value: 0 |
There was a problem hiding this comment.
golden_path.output schema/assertion do not match emitted event payload
Line 46 uses ModelGovernanceCheckCompleted, and Lines 48-50 assert exit_code == 0, but the emitter payload uses fields like check_type, target, passed, violation_count, and details (no exit_code). This will cause false contract expectations.
Suggested contract fix
output:
topic: "onex.evt.onex-change-control.governance-check-completed.v1"
output_correlation_id_field: "correlation_id"
- schema_name: "ModelGovernanceCheckCompleted"
+ schema_name: "GovernanceCheckCompleted" # replace with the actual existing model name
assertions:
- - field: "exit_code"
- op: "eq"
- value: 0
+ - field: "passed"
+ op: "eq"
+ value: true
+ - field: "violation_count"
+ op: "eq"
+ value: 0📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| schema_name: "ModelGovernanceCheckCompleted" | |
| assertions: | |
| - field: "exit_code" | |
| op: "eq" | |
| value: 0 | |
| schema_name: "GovernanceCheckCompleted" | |
| assertions: | |
| - field: "passed" | |
| op: "eq" | |
| value: true | |
| - field: "violation_count" | |
| op: "eq" | |
| value: 0 |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@contracts/OMN-5291.yaml` around lines 46 - 50, The contract currently
references schema_name "ModelGovernanceCheckCompleted" and asserts field
"exit_code == 0", but the emitter emits fields like "check_type", "target",
"passed", "violation_count", and "details"; update the schema_name and
assertions to match the actual payload — replace or rename
"ModelGovernanceCheckCompleted" if wrong and remove the "exit_code" assertion,
then add assertions for the real fields (e.g., assert "passed" is boolean or
true/false as expected, "violation_count" is a non-negative integer,
"check_type" and "target" are strings, and "details" exists/is an object or
array) using the exact field names from the emitter so the contract aligns with
emitted events.
| dod_evidence: | ||
| - id: "dod-001" | ||
| description: "Governance emitter unit tests pass" | ||
| source: "manual" | ||
| linear_dod_text: "Unit tests cover all three emit functions, verify topic names, envelope structure, | ||
| and silent no-op" | ||
| checks: | ||
| - check_type: "test_passes" | ||
| check_value: "uv run pytest tests/ -k 'emitter or governance' -v" | ||
| - id: "dod-002" | ||
| description: "mypy strict passes on kafka module" | ||
| source: "manual" | ||
| linear_dod_text: "mypy --strict passes on kafka/ module" | ||
| checks: | ||
| - check_type: "command" | ||
| check_value: "uv run mypy src/onex_change_control/kafka/ --strict" |
There was a problem hiding this comment.
Add explicit validate-yaml DoD evidence for schema compliance
The added dod_evidence only covers pytest and mypy. For this YAML artifact, include a validate-yaml CLI check against ModelTicketContract so contract compliance is explicitly evidenced.
Suggested DoD evidence addition
dod_evidence:
- id: "dod-001"
description: "Governance emitter unit tests pass"
@@
- id: "dod-002"
description: "mypy strict passes on kafka module"
@@
- check_type: "command"
check_value: "uv run mypy src/onex_change_control/kafka/ --strict"
+ - id: "dod-003"
+ description: "Contract validates via validate-yaml CLI"
+ source: "manual"
+ linear_dod_text: "OMN-5291 contract validates against ModelTicketContract"
+ checks:
+ - check_type: "command"
+ check_value: "uv run validate-yaml contracts/OMN-5291.yaml"As per coding guidelines, "{contracts,drift/day_close}/**/*.yaml: YAML artifact files must validate against the appropriate Pydantic schema (ModelTicketContract for ticket contracts, ModelDayClose for day close reports) using the validate-yaml CLI tool".
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| dod_evidence: | |
| - id: "dod-001" | |
| description: "Governance emitter unit tests pass" | |
| source: "manual" | |
| linear_dod_text: "Unit tests cover all three emit functions, verify topic names, envelope structure, | |
| and silent no-op" | |
| checks: | |
| - check_type: "test_passes" | |
| check_value: "uv run pytest tests/ -k 'emitter or governance' -v" | |
| - id: "dod-002" | |
| description: "mypy strict passes on kafka module" | |
| source: "manual" | |
| linear_dod_text: "mypy --strict passes on kafka/ module" | |
| checks: | |
| - check_type: "command" | |
| check_value: "uv run mypy src/onex_change_control/kafka/ --strict" | |
| dod_evidence: | |
| - id: "dod-001" | |
| description: "Governance emitter unit tests pass" | |
| source: "manual" | |
| linear_dod_text: "Unit tests cover all three emit functions, verify topic names, envelope structure, | |
| and silent no-op" | |
| checks: | |
| - check_type: "test_passes" | |
| check_value: "uv run pytest tests/ -k 'emitter or governance' -v" | |
| - id: "dod-002" | |
| description: "mypy strict passes on kafka module" | |
| source: "manual" | |
| linear_dod_text: "mypy --strict passes on kafka/ module" | |
| checks: | |
| - check_type: "command" | |
| check_value: "uv run mypy src/onex_change_control/kafka/ --strict" | |
| - id: "dod-003" | |
| description: "Contract validates via validate-yaml CLI" | |
| source: "manual" | |
| linear_dod_text: "OMN-5291 contract validates against ModelTicketContract" | |
| checks: | |
| - check_type: "command" | |
| check_value: "uv run validate-yaml contracts/OMN-5291.yaml" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@contracts/OMN-5291.yaml` around lines 53 - 68, Add a new dod_evidence entry
to this YAML to run the validate-yaml CLI against the appropriate Pydantic
schema: include a check with check_type "command" and check_value "uv run
validate-yaml --schema ModelTicketContract <path-to-this-file>" (reference the
existing dod_evidence list and the ticket contract schema ModelTicketContract)
so the contract file explicitly records schema validation alongside the pytest
and mypy checks.
…elf-extending-agent fixes (#1689) Adds OCC contracts and receipt evidence for two onex-self-extending-agent tickets: - OMN-11827: max_tokens truncation fix (PRs #136 merged, #138 open) - OMN-12124: clean ADK auth error handling (PR #137 open) These contracts unblock the receipt gate on those PRs — both reference OCC#<this PR>.
Summary
dod_evidencecontaining executable checks [OMN-7430]golden_pathto 5 seam tickets (OMN-3940, OMN-3944, OMN-3946, OMN-3947, OMN-5291)ModelTicketContractTest plan
ModelTicketContract.model_validate()Part of epic OMN-7426
🤖 Generated with Claude Code
Summary by CodeRabbit