Repository navigation
test(OMN-18125): fixture for the coverage corpus branch where provenance declares a binding - #3397
Conversation
…lare a binding Follow-up to omnibase_infra#3390, which squash-merged as 8e3cde8 while this commit was still in the governed pre-push. The rule change landed there; this is the fixture that exercises its other branch. OMN-16493 (closer run 34443923859, 2026-09-10T06:08:34Z) is not OMN-17478's shape. Its lane authored two structural self-binds carrying binds_ac, each with a derived ::pr-live-state twin, so four non-probative records land in the coverage denominator against six verified. It clears by two. A fixture built only on OMN-17478's shape, where every provenance item is unbound and the denominator is empty, never exercises that branch. The test asserts the denominator is exactly 4, not only the decision: a change that released this by emptying the corpus rather than scoping it would pass a decision-only test and would have reopened the padding loophole. The second test states the carry-forward rule as an assertion: removing the four bindings empties the denominator, so declaring a criterion on a structural self-bind only ever moves records INTO it. Never put binds_ac on a self-bind; autobind is right to omit it, and a merged contract cannot be corrected because the append-only gate is the authority.
There was a problem hiding this comment.
Hostile Reviewer — adversarial findings (OMN-17492)
Models succeeded: glm-review
Models failed: codex
New finding threads: 0
Deduped (already posted on this PR): 0
Nit-level findings suppressed: 1
The model is the FINDER, never the gate: merge is gated only by the
deterministic Hostile Review Thread Gate, which blocks while
hostile-reviewer threads are unresolved. Resolve each thread after
addressing (or rejecting, with a reply) its finding.
Findings not anchored to a changed file
-
[MAJOR] hostile-reviewer (glm-review)
Tests import private helper _coverage_corpus_counts | Both new tests reach into the handler module for an underscore-prefixed function. This couples the coverage corpus tests to an internal implementation detail with no stability contract; any refactor of the counting logic breaks these tests even when behavior is preserved, and conversely the tests can be green while the public decision path diverges from the helper. | Evidence: from omnibase_infra.nodes.node_evidence_autoclose_sweep_effect.handlers.handle
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MAJOR] hostile-reviewer (glm-review)
Boundary case of the two-item margin is untested | The docstrings state the shape 'clears by two' and warn that a fixture built only on the first shape 'never exercises that branch at all'. Yet the new tests assert only the passing side. There is no test that adding one more binds_ac-declaring non_probative record (denominator 5 against 6) fails, or that removing one verified record fails. The exact boundary behavior the branch encodes is never pinned; a strict-vs-greater-or-equal comparison error passes bo
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MINOR] hostile-reviewer (glm-review)
Fixture object aliasing between handler call and direct helper assertion | In the first new test, the same checks list is passed to _skill_result(checks) and later to _coverage_corpus_counts(verdict, ...). If the handler or any downstream code mutates the check dicts (e.g. strips or normalizes binds_ac), the second assertion measures post-handler state rather than the fixture as constructed, and can pass or fail for the wrong reason. | Evidence: result = await _handler(_skill_result(checks), linear).handle(
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MINOR] hostile-reviewer (glm-review)
Stripping test assumes checks are mutable plain dicts | The dict comprehension {k: v for k, v in check.items() if k != "binds_ac"} presumes _check returns a plain dict with string keys and no aliasing on values. If _check ever returns a model or immutable mapping, this test fails with an opaque AttributeError rather than an assertion. It also silently drops other keys' identities (tuple values are shared, harmless here but unstated). | Evidence: omitted = [
{k: v for k, v in check.items() if k != "bindsResolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MINOR] hostile-reviewer (glm-review)
Opaque tuple contract in _coverage_corpus_counts assertion | The assertion == (6, 4) encodes an undocumented positional tuple (verified, denominator) with no named unpacking. A reorder of the tuple elements that keeps the totals plausible (4, 6) would fail these tests only by accident of the specific numbers, and readers must consult the helper to decode the meaning the docstrings spend paragraphs explaining. | Evidence: assert _coverage_corpus_counts(verdict_declared, _FIVE_AC_DESCRIPTION) == (6, 4)
assertResolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MINOR] hostile-reviewer (glm-review)
Provenance metadata in docstrings is unverifiable and untested | The docstrings cite a specific run ID, commit hash, and timestamp (2026-09-10T06:08:34Z) as the basis for the fixture. Nothing in the test ties these numbers to the fixture contents; the counts (22 non_probative, denominator 4) are hand-transcribed with no cross-check, and the 'held at 6 probative' claim is baked into literals. If the transcription is wrong, the test enshrines the error. | Evidence: Measured on closer run 34443923859 at 2026-0
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492).
|
| Surface | Meaning | Blocks merge? |
|---|---|---|
| Review threads | Per-finding, posted by the reviewer | No (informational) |
Hostile Review Thread Gate |
Deterministic: unresolved hostile-reviewer threads exist | Fails until resolved (not yet a required context) |
degraded verdict |
Fewer than 2 models succeeded (infra) | No |
Powered by omniintelligence.review_pairing.cli_review — multi-model adversarial review: qwen3-review, qwen3-review-b, glm-review (OMN-8468/OMN-8524/OMN-17492)
There was a problem hiding this comment.
Hostile Reviewer — adversarial findings (OMN-17492)
Models succeeded: glm-review
Models failed: codex
New finding threads: 3
Deduped (already posted on this PR): 0
Nit-level findings suppressed: 1
The model is the FINDER, never the gate: merge is gated only by the
deterministic Hostile Review Thread Gate, which blocks while
hostile-reviewer threads are unresolved. Resolve each thread after
addressing (or rejecting, with a reply) its finding.
Findings not anchored to a changed file
-
[MAJOR] hostile-reviewer (glm-review)
Potential IndexError on result.outcomes[0] without guard | The test indexes result.outcomes[0] directly. If the handler returns an empty outcomes list under any code path reachable from this input (e.g., a sweep that short-circuits before evaluation), the test fails with IndexError rather than a meaningful assertion message. More importantly, the surrounding production contract for empty outcomes is not exercised by this diff at all, despite the fixture introducing a new denominator shape. | Evidence: resul
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MINOR] hostile-reviewer (glm-review)
Docstring claims RED status unverifiable in this diff | The test docstring asserts this was RED at the parent commit ('a SUBSTANTIVE red at the parent commit'). Nothing in the diff or the test itself substantiates or preserves that claim; once merged the test passes and the RED provenance is folklore. If the claim is operationally important to the contract narrative, it belongs in a comment on the ticket, not a docstring future readers cannot verify. | Evidence: """RED. Six verified against four bound non-p
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MINOR] hostile-reviewer (glm-review)
Second test duplicates first test's corpus assertion | test_a_structural_self_bind_declaring_a_criterion_only_ever_costs re-asserts _coverage_corpus_counts(verdict_declared, ...) == (6, 4), which the first new test already establishes. Redundant assertions on shared fixtures slow diagnosis: when the value changes, two tests fail for one cause, and the delta-specific assertion (6, 0) is buried among duplicates. | Evidence: assert _coverage_corpus_counts(verdict_declared, _FIVE_AC_DESCRIPTION) == (6, 4)
asserResolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492).
Findings demoted from threads (anchor rejected)
-
[MAJOR] hostile-reviewer (glm-review)
Tests import private helper _coverage_corpus_counts | Both new tests reach into the handler module for an underscore-prefixed private function. This couples the tests to an internal implementation detail rather than the public contract. A refactor that inlines, renames, or changes the signature of _coverage_corpus_counts breaks these tests even if observable behavior is unchanged, which is exactly the coupling unit tests exist to avoid. The tests should exercise the helper's effect through the handler's pub
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MINOR] hostile-reviewer (glm-review)
Magic-number fixture assertions duplicate fixture logic | assert total == 28, verified == 6, non_probative == 22 recompute properties of the fixture with independent literals. If _omn_16493_checks is edited, these constants must be hand-synchronized. The counts are already derived in the preceding comprehensions; asserting literals adds a second source of truth that will drift silently into a red herring failure unrelated to the behavior under test. | Evidence: total = len(checks)
verified = sum(1 for c inResolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MINOR] hostile-reviewer (glm-review)
Boundary between clears and fails untested | The new shape clears by two (6 verified vs 4 denominator). The diff adds no test for the adjacent failing shape, e.g., 5 verified against the same denominator, or 6 verified against a denominator of 7. Without a boundary case, a bug that decrements the denominator or inflates the numerator by one is undetected by this corpus; the stated purpose is coverage of the non-empty denominator branch, but only the passing side of it is exercised. | Evidence: It clears by
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492).
…ibase_infra#3397 (#8926) * evidence(OMN-18125): author OCC companion for OmniNode-ai/omnibase_infra#3397 OCC companion by node_pr_lifecycle_fix_effect (OMN-13317 F1 / OMN-13990 / OMN-14285). Product PR head e43b680227c42a2bce0cd6cccd7278728d47e9ef. * evidence(OMN-18125): self-bind OCC#8926 + rebind contract_sha256 --------- Co-authored-by: omnimarket-bot <bot@omninode.ai>
…lare a binding (#3397) Follow-up to omnibase_infra#3390, which squash-merged as 8e3cde8 while this commit was still in the governed pre-push. The rule change landed there; this is the fixture that exercises its other branch. OMN-16493 (closer run 34443923859, 2026-09-10T06:08:34Z) is not OMN-17478's shape. Its lane authored two structural self-binds carrying binds_ac, each with a derived ::pr-live-state twin, so four non-probative records land in the coverage denominator against six verified. It clears by two. A fixture built only on OMN-17478's shape, where every provenance item is unbound and the denominator is empty, never exercises that branch. The test asserts the denominator is exactly 4, not only the decision: a change that released this by emptying the corpus rather than scoping it would pass a decision-only test and would have reopened the padding loophole. The second test states the carry-forward rule as an assertion: removing the four bindings empties the denominator, so declaring a criterion on a structural self-bind only ever moves records INTO it. Never put binds_ac on a self-bind; autobind is right to omit it, and a merged contract cannot be corrected because the append-only gate is the authority.
Evidence-Source: OCC#8926
Evidence-Ticket: OMN-18125
OMN-18125, follow-up fixture
The rule change landed in omnibase_infra#3390, squash
8e3cde8bcd8ed37e7b53ed5c61bf7868d8caad2e, merged 2026-09-10T09:37:20Z. This commit was still inside the governed pre-push at that moment, so the fixture it adds did not go with it. This PR carries only that fixture. No production code changes.What it exercises that #3390 did not
OMN-16493, closer run 34443923859 at 2026-09-10T06:08:34Z, is a different shape from OMN-17478.
On OMN-17478 every one of the 22 provenance entries was unbound, so the coverage denominator was empty and the scoped rule released with room to spare. OMN-16493's lane authored two structural self-binds that DO carry
binds_ac,occ-self-bind-pr-8858andocc-self-bind-pr-8876, and each brought a derived::pr-live-statetwin. Four non-probative records therefore land in the denominator against six verified. It clears by two.test_the_omn_16493_shape_clears_with_a_denominator_of_exactly_fourasserts that denominator directly, not only the decision. A change that released this case by emptying the corpus rather than by scoping it would pass a decision-only test and would have reopened the padding loophole the rule exists to close.RED, and it is substantive
Both new tests were run against the parent tree with
bcb3e6460's version of the node checked out in place. The decision assertion is deliberately ordered ahead of the helper import so this case fails on the verdict rather than on anImportErrorfor a helper that did not exist yet:The whole module at that parent tree read
7 failed, 4 passed. On mergeddevit reads11 passed, and the full closer suite reads397 passed.Carry-forward rule, asserted rather than written down
Never put
binds_acon a structural self-bind. Autobind is right to omit it. A self-bind is a baregh pr view, so it can never verify, so it can never enter the numerator; declaring a criterion on it moves it from outside the coverage corpus into the denominator and nowhere else. It makes the contract that declares it harder to close, never easier.test_a_structural_self_bind_declaring_a_criterion_only_ever_costsmeasures exactly that: removing the four bindings takes the denominator from 4 to 0.This is a rule for the next contract, not a repair for the last one. A merged contract cannot be corrected, because the append-only gate is the authority. No merged evidence item was edited by this PR or by #3390.
Verification
ruff format --checkandruff checkclean on the test tree. No production module changed, so there is nothing new formypy --strictto see beyond what fix(OMN-18125): the coverage rule judges the checks that claim coverage #3390 already proved clean.evidence-autoclose-sweepworkflow, not on the.201compose dev lane or theonex-laboverlay. No lab pass exercises it and none is claimed.