Repository navigation
fix(OMN-16823): recognize CodeRabbit's agreement-plus-deferral concession class in the CR-thread gate - #2057
Merged
Merged
Conversation
…sion class check-unresolved-threads.sh counted a CodeRabbit thread as blocking even after CodeRabbit conceded the finding in-thread and agreed to defer it to a ticket. Its concession regex only matched "you're right" / "apologize" / "correct behavior" / "retract" / "understood ... reasonable" — not the phrasing CodeRabbit actually used on omnibase_core#1604 thread 2 of 3: "agreed that `EnumLogLevel` cannot represent this YAML contract ... deferring the validation design to OMN-16037 is reasonable" The required `gate / CodeRabbit Thread Check` context was therefore unsatisfiable by construction: no path to green short of making a change the reviewer itself had agreed was not required. DETECTION change only. The widened class is a conjunction — an agreement token AND a deferral/acceptance token AND no negation-or-escalation marker — matched over the SALIENT reply body (auto-generated <details> blocks and HTML comments stripped, so CodeRabbit's own "Learnings" boilerplate cannot supply either token). The pre-existing human-rebuttal precondition and the truncated-thread conservatism both still gate it, so the check cannot degrade into "any CodeRabbit reply clears the thread". Also collapses the previously byte-duplicated concession regex in CONCESSION_JQ and BLOCKING_JQ into one shared CR_CONCEDED_JQ predicate — the duplication is how a widened match lands in the audit line but not in the count. Enforcement (Rule 5): the filter suite now runs as a pre-commit hook AND as a step in the CI `quality` job, which feeds the required `quality-gate` context. It was previously an opt-in script wired to nothing. TDD red-first: Case K (the verbatim #1604 reply) failed 1 != 0 before the fix. Permanent negative controls added, all proven by execution: L unanswered CodeRabbit finding -> blocking M CodeRabbit escalates using the deferral vocabulary -> blocking N "do not agree that deferring ... is reasonable" -> blocking O agreement on a sub-point, finding kept -> blocking P tokens only inside a <details> learnings block -> blocking Q agreement-plus-deferral with no human rebuttal -> blocking R truncated thread with a concession in the slice -> blocking No auto-resolution is introduced. `pr_review_bot` remains the sole resolver of CodeRabbit threads; the suite asserts the script contains no resolve mutation. Refs: OMN-16823
The jq filter suite was reachable only from a bash invocation nobody made. It now also runs as a pytest unit test, so the default local test run covers it, and — because `pytest` is a BEHAVIOR-class command head for node_dod_verify — the OCC evidence for this ticket can be behavior-proving rather than a static file read. The wrapper executes the real suite; it does not re-implement the filters. It additionally asserts by name that the OMN-16823 positive case (the verbatim omnibase_core#1604 concession) and all seven permanent negative controls are still present, so a regression that quietly deletes a control fails here even if the shell suite's own exit code were loosened. Refs: OMN-16823
|
Warning Review limit reachedNext included review available in 42 minutes. View limit detailsLimit details: You’ve used the included review currently available. Your 132 included PR review attempts over the past 7 days set your current allowance at 1 review per hour. Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. Review configuration: ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
Comment |
jonahgabriel
pushed a commit
to OmniNode-ai/onex_change_control
that referenced
this pull request
Aug 28, 2026
…iclaude#2057 (#7390) * evidence(OMN-16823): author OCC companion for OmniNode-ai/omniclaude#2057 OCC companion by node_pr_lifecycle_fix_effect (OMN-13317 F1 / OMN-13990 / OMN-14285). Product PR head 7af10ac426e254904be523b5ce8a2b7aada67d53. * evidence(OMN-16823): self-bind OCC#7390 + rebind contract_sha256 --------- Co-authored-by: omnimarket-bot <bot@omninode.ai>
Contributor
|
| Verdict | Meaning | Blocks merge? |
|---|---|---|
passed |
No critical/error findings | No |
blocked |
CRITICAL or ERROR findings found | Yes |
degraded |
Reviewer ran but every model failed mid-run | No (pilot) |
infra_error |
cli_review failed to run at all (install/crash) after 2 attempts | Yes |
Powered by omniintelligence.review_pairing.cli_review — node-based adversarial review via HandlerLlmCliSubprocess (OMN-8524)
This was referenced Sep 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes OMN-16823
Problem
scripts/check-unresolved-threads.shbacks the requiredgate / CodeRabbit Thread Checkcontext. It counted a CodeRabbit review thread as blocking even after CodeRabbit had conceded the finding in-thread and agreed to defer it to a ticket, because its concession match only recognizedyou're right/apologize/correct behavior/retract/understood ... reasonable.The live case is omnibase_core#1604, thread 2 of 3 on
src/omnibase_core/models/cli/model_cli_user_config_logging.py. CodeRabbit's own reply, verbatim:An unambiguous concession plus an explicit deferral — and the required context stayed red, with no path to green short of making a change the reviewer itself agreed was not required. Same family as OMN-15815 / OMN-15532: a required context unsatisfiable by construction.
Fix — detection only
The widened class is a conjunction, not a keyword list:
agreed,agree,concur,fair point, ...), ANDdefer*,reasonable,pragmatic,tradeoff,out of scope,separate ticket,OMN-####, ...), ANDdisagree,do not agree,still stands,reiterate,escalate,finding stands, ...),evaluated over the salient reply body — auto-generated
<details>blocks and HTML comments are stripped first, so CodeRabbit's own "Learnings" boilerplate cannot supply either token. The pre-existing human-rebuttal precondition and the truncated-thread conservatism both still gate it. The gate cannot degrade into "any CodeRabbit reply clears the thread".Also collapses the previously byte-duplicated concession regex in
CONCESSION_JQandBLOCKING_JQinto one sharedCR_CONCEDED_JQpredicate. That duplication is precisely how a widened match lands in the audit line but not in the count.No auto-resolution —
pr_review_botremains the sole resolverThis change introduces no auto-resolution and no resolve path of any kind. It changes only the wording match that decides whether a thread still counts. Asserted mechanically, not just in prose: the suite greps the gate script for
resolveReviewThread/unresolveReviewThread/--resolveand fails if any appears (PASS: no thread-resolution mutation present in the gate script). No[skip-*]token, no bypass, no required-check removal is used anywhere in this change.Evidence — TDD red-first, negative controls proven by execution
RED before the fix, with the verbatim #1604 reply as the fixture:
GREEN after, 24 assertions. The OMN-16823 surface:
Cases L–R are the permanent negative controls required by AC3, each proven red by execution rather than argued. M is the one a bare keyword search gets wrong: CodeRabbit escalating while reusing the entire deferral vocabulary ("I disagree that deferring the validation design to OMN-16037 is reasonable"). P proves the salient-body stripping: the concession vocabulary sits only inside a
<details>Learnings</details>block while the visible reply keeps the finding.Falsifiability is by discrimination, not assertion: restoring only
scripts/check-unresolved-threads.shto itsorigin/devcontent in the same worktree and re-running reproducesFAIL Case K: expected 0 got 1.Enforcement (Operating Rule 5)
The filter suite was an opt-in script wired to nothing. It now runs in two enforced places:
cr-thread-gate-filters(verified firing:CR-thread-gate jq filter tests (OMN-16823)....Passed)qualityjob, which feeds the requiredquality-gatecontextplus
tests/scripts/test_check_unresolved_threads_filters.py, a pytest entrypoint that executes the real suite and additionally asserts by name that the positive case and all seven negative controls are still present — so a regression that quietly deletes a control fails rather than silently passing.dod_evidence
OCC evidence companion:
contracts/OMN-16823.yaml+drift/dod_receipts/OMN-16823/dod-omn-16823-concession-detection-behavior/inonex_change_control(merges first). Both bound checks are BEHAVIOR class and both execute the filters rather than reading a file.Evidence-Ticket: OMN-16823
Evidence-Source: OCC#7390