Repository navigation
fix(OMN-19941): INDETERMINATE fails closed in every verdict path - #4274
jonahgabriel merged 3 commits into
Conversation
…rdict, and INDETERMINATE is pinned failing in the pr-head verifier Onex-Lane: regr-a7-2330 Onex-Session: 0087a41b0dd9467bac680b04d3ba0934
…9957, OMN-19958, OMN-19959, OMN-19960, OMN-19961): author OCC companion for OmniNode-ai/omnibase_infra#4274 OCC companion by node_pr_lifecycle_fix_effect (OMN-13317 F1 / OMN-13990 / OMN-14285). Product PR head 9380db5bb82ae900253a713b6df46287a53b780c.
…9957, OMN-19958, OMN-19959, OMN-19960, OMN-19961): OCC batch window for OmniNode-ai/omnibase_infra (#11757) * evidence(OMN-19941, OMN-19399, OMN-19407, OMN-19513, OMN-19566, OMN-19957, OMN-19958, OMN-19959, OMN-19960, OMN-19961): author OCC companion for OmniNode-ai/omnibase_infra#4274 OCC companion by node_pr_lifecycle_fix_effect (OMN-13317 F1 / OMN-13990 / OMN-14285). Product PR head 9380db5bb82ae900253a713b6df46287a53b780c. * evidence(OMN-19941, OMN-19399, OMN-19407, OMN-19513, OMN-19566, OMN-19957, OMN-19958, OMN-19959, OMN-19960, OMN-19961): self-bind OCC#11757 + rebind contract_sha256 * evidence(OMN-19399): yamlfmt the regenerated OCC batch-window contracts and rebind their whole-file hashes The regenerated batch-window contracts for omnibase_infra#4268, #4264, #4267, #4269 and #4271 end without a trailing newline, so the yamlfmt hook rewrites them in CI and fails Pre-commit. Add the newline and rebind each receipt's contract_sha256 to the formatted bytes. The producer fix is omnimarket#3052. Onex-Lane: worktree-cleanup-auto-83 Onex-Session: 301518c27b4a4717b097d4c72bb6145b --------- Co-authored-by: omnimarket-bot <bot@omninode.ai> Co-authored-by: jonahgabriel <jonah@omninode.ai>
There was a problem hiding this comment.
Hostile Reviewer — adversarial findings (OMN-17492)
Models succeeded: qwen3-review, gpt-oss-review
Models failed: none
New finding threads: 0
Deduped (already posted on this PR): 0
Below quorum (one model only, reported not threaded): 7
Nit-level findings suppressed: 0
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.
Below quorum: 7 finding(s) raised by one model only (OMN-18479)
These are reported and NOT dropped, but they get no thread and do not block: a single model's finding no other model reproduced is not evidence enough to stop a merge. Read them; act on them if they are right.
- [MAJOR]
src/omnibase_infra/nodes/node_lab_proof_plan_compute/handlers/handler_lab_proof_verdict.py(qwen3-review) — Missing observation check for non-focused mandatory steps | The diff adds a fail-closed check for FOCUSED_TESTS when the step is planned but has no observation. However, the same logic is not applied - [MAJOR]
tests/unit/ci/test_indeterminate_fails_closed_omn19941.py(qwen3-review) — Bypassing dataclass validation in tests may mask real-world inconsistencies | The new test test_indeterminate_mandatory_check_with_forced_pass_result_is_refused uses object.setattr to bypass froze - [MAJOR]
src/omnibase_infra/nodes/node_lab_proof_plan_compute/handlers/handler_lab_proof_verdict.py:306-307(gpt-oss-review) — Identity comparison for step identifiers may misclassify presence of focused tests | The new check usesstep.step_id is _ID.FOCUSED_TESTSto determine whether the plan contains a focused‑tests step. - [MAJOR]
src/omnibase_infra/nodes/node_lab_proof_plan_compute/handlers/handler_lab_proof_verdict.py:145-148(gpt-oss-review) — Missing null‑check forplanargument may raise AttributeError | The handler now forwardsplanto_checkwithout verifying thatplanis non‑null. If a request omits the plan (e.g., malformed in - [MAJOR]
src/omnibase_infra/nodes/node_lab_proof_plan_compute/handlers/handler_lab_proof_verdict.py:285-310(gpt-oss-review) — Changed_checksignature breaks external callers | The method_checkpreviously accepted(check, report)and is now defined as(check, plan, report). Only the internal call in this file was up - [MINOR]
tests/unit/nodes/node_lab_proof_plan_compute/test_handler_lab_proof_verdict.py(qwen3-review) — Test for 'not-run' scenario does not verify specific failure reason | The test test_mandatory_step_not_run_or_timed_out_never_passes asserts that the check fails and the outcome is not PASS, but it do - [MINOR]
src/omnibase_infra/nodes/node_lab_proof_plan_compute/handlers/handler_lab_proof_verdict.py(qwen3-review) — Tight coupling between plan and report in _check method | The _check method now takes both plan and report as arguments. This creates a tight coupling between the two models, which may make it difficu
✅ Hostile Reviewer — REVIEWEDCritical findings: 0
Semantics (OMN-17492 — the model finds, thread resolution gates)
Powered by omniintelligence.review_pairing.cli_review — multi-model adversarial review: qwen3-review (Qwen3.8-27B), gpt-oss-review (gpt-oss-120b) (OMN-8468/OMN-8524/OMN-17492) |
…sed verdict The contract sync gate requires a handler change to touch its node contract. The verdict handler now fails a planned but unobserved step closed; the contract description and ticket list say so. Onex-Lane: regr-a7-2330 Onex-Session: 0087a41b0dd9467bac680b04d3ba0934
There was a problem hiding this comment.
Hostile Reviewer — adversarial findings (OMN-17492)
Models succeeded: qwen3-review, gpt-oss-review
Models failed: none
New finding threads: 2
Deduped (already posted on this PR): 0
Below quorum (one model only, reported not threaded): 5
Nit-level findings suppressed: 2
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.
Below quorum: 5 finding(s) raised by one model only (OMN-18479)
These are reported and NOT dropped, but they get no thread and do not block: a single model's finding no other model reproduced is not evidence enough to stop a merge. Read them; act on them if they are right.
- [MINOR]
tests/unit/ci/test_indeterminate_fails_closed_omn19941.py(qwen3-review) — Bypassing dataclass validation in test may mask real-world inconsistencies | The testtest_indeterminate_mandatory_check_with_forced_pass_result_is_refuseduses `object.setattr(receipt, "result" - [MINOR]
src/omnibase_infra/nodes/node_lab_proof_plan_compute/contract.yaml(qwen3-review) — No test for the case where a planned step is not in the report but the check is not mandatory | The new code path in_checkis only triggered for mandatory checks, as it is called fromhandlewhic - [MINOR]
src/omnibase_infra/nodes/node_lab_proof_plan_compute/handlers/handler_lab_proof_verdict.py(gpt-oss-review) — Repeated linear scan of plan steps per mandatory check | Each invocation of _check iterates over plan.steps with any() to locate the FOCUSED_TESTS step, resulting in O(number_of_steps) work for every - [MINOR]
tests/unit/nodes/node_lab_proof_plan_compute/test_handler_lab_proof_verdict.py(gpt-oss-review) — No test for extra observation when plan omits focused_tests step | The new test suite validates the failure path when a planned focused_tests step lacks an observation and when mandatory steps are not - [MINOR]
src/omnibase_infra/nodes/node_lab_proof_plan_compute/handlers/handler_lab_proof_verdict.py(gpt-oss-review) — Increased coupling of _check to full plan object | The _check method now requires the entire ModelLabProofPlan, whereas previously it operated solely on the report. This tightens the dependency betwee
Findings demoted from threads (anchor rejected)
-
[MAJOR] hostile-reviewer (qwen3-review)
Missing observation for a planned step is conflated with a failed step | The new code path in
_checkforFOCUSED_TESTSreturnsresult(False, "undecidable: planned focused_tests has no observation")whenreport.get(_ID.FOCUSED_TESTS) is None. This treats the absence of an observation as a definitive failure of the check. However, the testtest_mandatory_step_not_run_or_timed_out_never_passesexplicitly setsran=Falseandtimed_out=Truewithexit_code=Noneandexpectation_met=False, implyingResolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MAJOR] hostile-reviewer (gpt-oss-review)
Missing observation handling for non‑focused mandatory checks | The diff adds special handling for missing observations only for the FOCUSED_TESTS check. Other mandatory checks that may rely on plan steps are not given analogous handling, so a plan that includes a step but lacks its observation could incorrectly pass or fail the verdict. | Evidence: In handler_lab_proof_verdict.py the new logic checks "if not any(step.step_id is _ID.FOCUSED_TESTS for step in plan.steps):" and then handles the missing observ
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492).
There was a problem hiding this comment.
Hostile Reviewer — adversarial findings (OMN-17492)
Models succeeded: qwen3-review, gpt-oss-review
Models failed: none
New finding threads: 2
Deduped (already posted on this PR): 0
Below quorum (one model only, reported not threaded): 6
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.
Below quorum: 6 finding(s) raised by one model only (OMN-18479)
These are reported and NOT dropped, but they get no thread and do not block: a single model's finding no other model reproduced is not evidence enough to stop a merge. Read them; act on them if they are right.
- [MAJOR]
tests/unit/ci/test_indeterminate_fails_closed_omn19941.py(qwen3-review) — Test bypasses validation via object.setattr, masking potential model inconsistencies | The testtest_indeterminate_mandatory_check_with_forced_pass_result_is_refuseduses `object.setattr(rec - [MINOR]
tests/unit/nodes/node_lab_proof_plan_compute/test_handler_lab_proof_verdict.py(qwen3-review) — No test for the case where FOCUSED_TESTS is not in the plan but is in mandatory_checks | The code handles the case where FOCUSED_TESTS is not in the plan by returning 'n/a'. However, there is no test - [MINOR]
src/omnibase_infra/nodes/node_lab_proof_plan_compute/handlers/handler_lab_proof_verdict.py(gpt-oss-review) — Enum comparison using identity operator | The new check uses 'step.step_id is _ID.FOCUSED_TESTS' to detect the presence of a focused_tests step. Enum members are singletons only when imported from the - [MINOR]
src/omnibase_infra/nodes/node_lab_proof_plan_compute/handlers/handler_lab_proof_verdict.py(gpt-oss-review) — Tight coupling of _check to plan structure | Introducing a 'plan' parameter into the private _check method couples check logic to the plan representation, violating the original design where checks op - [MINOR]
src/omnibase_infra/nodes/node_lab_proof_plan_compute/handlers/handler_lab_proof_verdict.py(gpt-oss-review) — Linear scan of plan steps for each focused test check | The added any() iteration over plan.steps executes for every invocation of the FOCUSED_TESTS check, introducing O(N) work where N is the number - [MINOR]
tests/unit/nodes/node_lab_proof_plan_compute/test_handler_lab_proof_verdict.py(gpt-oss-review) — Missing test for undefined or malformed plan input | The new logic assumes a well‑formed ModelLabProofPlan with a 'steps' attribute. No test exercises the handler when the plan is None or missing the
Findings demoted from threads (anchor rejected)
-
[MAJOR] hostile-reviewer (qwen3-review)
Missing observation for non-FOCUSED_TESTS planned steps is not handled | The diff adds a fail-closed check only for FOCUSED_TESTS: if a planned step has no observation, it returns a failed check. However, the same logic gap exists for other mandatory check types (e.g., SUBJECT_HEAD_IDENTITY, MIGRATION_GATE_HEALTHY). If a planned step for those checks has no observation, the code falls through to
single(_ID.SUBJECT_REV)or similar, which will likely return a passed check with no actual observation data. ThResolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MAJOR] hostile-reviewer (gpt-oss-review)
Potential undefined variable 'plan' in handle method | The call to self._check now includes a 'plan' argument, but the surrounding code snippet does not show a definition or assignment for a variable named 'plan' within the handle method's scope. If 'plan' is not defined earlier in the method, this will raise a NameError at runtime, breaking the handler. | Evidence: Line changed to: self._relative_to_base(self._check(check, plan, report), base, reasons) | Fix: Ensure that a variable 'plan' is obtained from
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492).
…ation-database gate)
There was a problem hiding this comment.
Hostile Reviewer — adversarial findings (OMN-17492)
Models succeeded: qwen3-review, gpt-oss-review
Models failed: none
New finding threads: 2
Deduped (already posted on this PR): 0
Below quorum (one model only, reported not threaded): 7
Nit-level findings suppressed: 0
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.
Below quorum: 7 finding(s) raised by one model only (OMN-18479)
These are reported and NOT dropped, but they get no thread and do not block: a single model's finding no other model reproduced is not evidence enough to stop a merge. Read them; act on them if they are right.
- [MAJOR]
tests/unit/ci/test_indeterminate_fails_closed_omn19941.py(qwen3-review) — Test bypasses dataclass validation via object.setattr | test_indeterminate_mandatory_check_with_forced_pass_result_is_refused uses object.setattr(receipt, "result", ...) to mutate a frozen dat - [MAJOR]
tests/unit/nodes/node_lab_proof_plan_compute/test_handler_lab_proof_verdict.py(qwen3-review) — No test for undecidable check with base-control present | The new fail-closed behavior for a missing FOCUSED_TESTS observation is tested only in the absence of a base-control result. There is no test - [MAJOR]
src/omnibase_infra/nodes/node_lab_proof_plan_compute/handlers/handler_lab_proof_verdict.py(gpt-oss-review) — Verdict handler now tightly coupled to plan model | The handler_lab_proof_verdict now imports ModelLabProofPlan and passes the entire plan into the _check method, creating a direct dependency between - [MINOR]
src/omnibase_infra/nodes/node_lab_proof_plan_compute/handlers/handler_lab_proof_verdict.py(qwen3-review) — Plan object passed to _check but only used for one check type | The _check method signature was extended to accept a ModelLabProofPlan, but the plan is only consulted for the FOCUSED_TESTS check. All - [MINOR]
tests/unit/nodes/node_lab_proof_plan_compute/test_handler_lab_proof_verdict.py(qwen3-review) — Parametrized test conflates two distinct failure modes | test_mandatory_step_not_run_or_timed_out_never_passes parametrizes over (ran=False, timed_out=False) and (ran=True, timed_out=True). The first - [MINOR]
src/omnibase_infra/nodes/node_lab_proof_plan_compute/handlers/handler_lab_proof_verdict.py(gpt-oss-review) — Identity comparison used for step identifiers | The code uses 'is' to compare step.step_id with _ID.FOCUSED_TESTS. If step_id is not an Enum singleton (e.g., a string or a newly constructed Enum membe - [MINOR]
src/omnibase_infra/nodes/node_lab_proof_plan_compute/handlers/handler_lab_proof_verdict.py(gpt-oss-review) — Repeated linear scan of plan steps per mandatory check | Each invocation of _check for the FOCUSED_TESTS case performs an O(N) scan over plan.steps to detect the presence of the step. If many mandator
Findings demoted from threads (anchor rejected)
-
[MAJOR] hostile-reviewer (qwen3-review)
Undecidable verdict check ignores base-control override | The new fail-closed branch in _check for FOCUSED_TESTS returns result(False, "undecidable: planned focused_tests has no observation") before the _relative_to_base wrapper can apply the DEV_INHERITED transformation. If a base-control run also lacks the observation, the check should be transformed to DEV_INHERITED per the documented contract, but instead it remains a hard FAIL. This breaks the documented invariant that a FAIL reproduced at the merge ba
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MAJOR] hostile-reviewer (gpt-oss-review)
Potential AttributeError when plan is missing | The _check method assumes the 'plan' argument is non‑null and accesses plan.steps. If a request lacks a plan (e.g., malformed input), the handler will raise an AttributeError, causing an unhandled exception and breaking the node execution. | Evidence: def _check(self, check: EnumLabProofCheck, plan: ModelLabProofPlan, report: ModelLabProofRunReport): ... any(step.step_id is _ID.FOCUSED_TESTS for step in plan.steps) | Fix: Validate that 'plan' is not None befor
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492).
Summary
Ticket OMN-19941 (A7 of the verification-and-declared-lab-state plan, parent OMN-17427): an undecidable result fails closed in the verdict compute and in the pr-head verifier.
lab_proof.verdict): thefocused_testscheck returned a vacuous pass ("n/a") whenever the report had no observation for that step, even when the plan contained the step. A planned but unobserved step is undecidable, so it now fails. "n/a" applies only when the plan has no such step.Acceptance criteria
test_planned_focused_tests_without_an_observation_fail_closedand the not-run/timeout parametrized test intest_handler_lab_proof_verdict.py. RED before the fix (1 failed), green after.tests/unit/ci/test_indeterminate_fails_closed_omn19941.py.dod_evidence
uv run pytest tests/unit/nodes/node_lab_proof_plan_compute tests/unit/ci/test_indeterminate_fails_closed_omn19941.py: 52 passed.single()already makes).Lab readback
Pure compute handler, no runtime wiring, no topics, no schema change. No hand lab run was made; the automatic post-merge lab pass on the merged sha (rule 24a) is the lab evidence. Tenant/RLS paths: not touched.
Evidence-Ticket: OMN-19941
Evidence-Source: OCC#11757
Lab pool readback (OMN-18893)