Repository navigation
fix(contracts): OMN-8606 DoD uses {pr}/{repo} placeholders, removes open-PR-blocking checks - #273
Conversation
…nd file-based checks
Replace bare `gh pr view` commands (fail in detached HEAD) with:
- dod-001: gh pr view {pr} --repo {repo} (uses CI-injected PR number)
- dod-002/003: file_exists + grep checks that pass on open PRs
- dod-004: pre-commit unchanged
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 0 minutes and 14 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the 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 configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (243)
📝 WalkthroughWalkthroughUpdated three evidence checks in Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
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: 1
🧹 Nitpick comments (1)
contracts/OMN-8606.yaml (1)
30-36: Evidence check is weaker than the stated requirement.Lines 30-36 validate only
prompt_tokens(Line 35), but the description says token columns. Consider adding another check (or a single command check) to assert all required token fields.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@contracts/OMN-8606.yaml` around lines 30 - 36, The check in OMN-8606.yaml currently only greps for "prompt_tokens" in migrations/0063_llm_routing_decisions.sql but the description requires token columns (plural); update the checks array to assert all required token fields (e.g., "prompt_tokens", "completion_tokens", "total_tokens" or whatever the schema uses) by adding additional grep checks or a single pattern that matches all token column names, referencing the existing check_type "grep" and the target file "migrations/0063_llm_routing_decisions.sql" so the contract verifies every token column rather than just prompt_tokens.
🤖 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-8606.yaml`:
- Around line 27-28: The contract uses an unsupported check_type "file_exists"
causing dod-002 to hard-fail; replace it with a supported check_type (e.g.,
"command" or "grep") that verifies the same condition. Update the entry where
check_type is "file_exists" and check_value is
"migrations/0056_session_post_mortems.sql" to use a supported check (for
example, set check_type to "command" and provide a command that tests file
presence, or set check_type to "grep" and use a pattern against an
index/listing) so the runner can execute the check without blocking.
---
Nitpick comments:
In `@contracts/OMN-8606.yaml`:
- Around line 30-36: The check in OMN-8606.yaml currently only greps for
"prompt_tokens" in migrations/0063_llm_routing_decisions.sql but the description
requires token columns (plural); update the checks array to assert all required
token fields (e.g., "prompt_tokens", "completion_tokens", "total_tokens" or
whatever the schema uses) by adding additional grep checks or a single pattern
that matches all token column names, referencing the existing check_type "grep"
and the target file "migrations/0063_llm_routing_decisions.sql" so the contract
verifies every token column rather than just prompt_tokens.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
The grep check_type runner uses --include=*.py which skips YAML files.
Replace with a command check that greps the contract YAML directly.
Pattern avoids {pr}/{repo} placeholders to prevent runner substitution.
…tracts DoD runner only handles endpoint/command/grep check types. Replace check_type: "file_exists" with check_type: "command" + command: "test -f <path>" across all 243 affected contract YAML files.
Summary
gh pr view {pr} --repo {repo}so the command resolves in CI detached HEADgh pr view --json mergedAtandgh pr checks --watch(always fail for open PRs) withfile_exists+grepchecks that verify the actual code changeWhy
Contract Compliance Check for omnidash PR #598 was failing because bare
gh pr viewcalls fail when git is in detached HEAD (as in CI merge checkouts), and dod-002 checked formergedAtwhich is null on open PRs.Test plan
Summary by CodeRabbit