Repository navigation
feat(OMN-14222): ARCH-005 orchestrator/reducer handler state-invariant CI gate - #2375
Conversation
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 36 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
…t CI gate Adds a new zero-tolerance static gate (scanner_orchestrator_reducer_state_invariant.py) that fails on any ClassVar[<mutable container>] or module-level mutable container (dict/list/set/OrderedDict/defaultdict/Counter) inside a handlers/*.py file belonging to a node_type: ORCHESTRATOR*/*REDUCER* contract, unless cleared with an inline `# orchestrator-reducer-state-ok: <reason>` comment. Unlike the sibling ARCH-004 Signal A/B ratchets, this rule targets BOTH orchestrators and reducers (reducers are not exempt) and carries no ratchet baseline: a live full-repo scan found exactly one pre-existing occurrence (_TRANSITIONS, a static FSM transition table in node_chain_verify_reducer/handlers/handler_chain_verify.py), which is cleared with the exemption comment rather than deleted or baselined, so the gate ships as a hard block from day one. Wired as both a blocking pre-commit hook (onex-orchestrator-reducer-state-invariant) and a blocking CI step (ARCH-005) in the same PR per Operating Rule #5 (enforcement, not detection). Scope note: this ticket's second deliverable -- converting the non-canonical ServiceSavingsEstimator Kafka consumer (omnibase_infra/services/observability/ savings_estimation/consumer.py) into a canonical NODE (EFFECT + DB-backed REDUCER) -- is NOT included in this PR. That conversion touches a live production Kafka consumer wired into service_kernel.py, requires new DB schema/migration and node/contract design decisions the ticket itself flags as open questions, and sits entirely outside the node/handler directory tree this gate scans (so the gate does not, and structurally cannot, catch it). Attempting that conversion in the same mechanical pass as the CI gate risked a regression in a real revenue-metrics pipeline without dedicated review. Recommend routing it through a dedicated design/build lane. RED: scanning the pre-fix handler_chain_verify.py blob (git show HEAD~) reproduces the ticket's one live violation. GREEN: post-fix full-repo scan is 0/26 target node dirs; 13 new unit tests (module-level + ClassVar detection, exemption clearing, EFFECT/COMPUTE out-of-scope, live-repo full audit) all pass. Fixes a non-optional-union ratchet regression the first draft introduced (ast.Assign | ast.AnnAssign param) by re-typing to the common ast.stmt base. Closes OMN-14222 (CI-gate deliverable only; ServiceSavingsEstimator node conversion tracked separately -- see PR body).
4a19853 to
2367318
Compare
Summary
Addresses OMN-14222 (CI-gate deliverable). Adds
ARCH-005, a new zero-tolerance static gate that fails on anyClassVar[<mutable container>]or module-level mutable container (dict/list/set/OrderedDict/defaultdict/Counter) inside ahandlers/*.pyfile belonging to anode_type: ORCHESTRATOR*/*REDUCER*contract, per the declarative-orchestrator/reducer state-persistence invariant surfaced by the OMN-14208 census (operator-confirmed 2026-07-09).src/omnibase_infra/nodes/node_architecture_validator/validators/scanner_orchestrator_reducer_state_invariant.py, modeled on the ARCH-004 Signal A/B scanner precedent (discover_node_dirs/node_dirs_for_changed_filesreused), but DISTINCT: reducers are NOT exempt here (Signal A/B exempt them), and there is no ratchet baseline — a live full-repo scan found exactly one pre-existing occurrence and it is cleared inline, not baselined._TRANSITIONSinnode_chain_verify_reducer/handlers/handler_chain_verify.py— a static, read-only FSM transition table, never mutated — via an inline# orchestrator-reducer-state-ok: <reason>exemption comment (the escape hatch the ticket specifies), not deletion or a baseline entry.onex-orchestrator-reducer-state-invariant) and a blocking CI step (nocontinue-on-error, unlike Signal A/B) in this same PR, per Operating Rule feat: Complete Phase 2 infrastructure migration to ONEX nodes #5 (enforcement, not detection).ClassVardetection, exemption clearing,__all__always-exempt, EFFECT/COMPUTE nodes out of scope, and a live-repo full-audit assertion (zero findings post-fix).Scope note —
ServiceSavingsEstimatorconversion NOT includedOMN-14222 has a second deliverable: converting the non-canonical
ServiceSavingsEstimatorKafka consumer (omnibase_infra/services/observability/savings_estimation/consumer.py) into a canonical NODE (EFFECT + DB-backed REDUCER holding the session-correlation buffer as atenant_id-scoped row). That conversion is not in this PR:service_kernel.py, feeding thesavings-estimated.v1pipeline._TRANSITIONSfinding.Attempting that conversion in the same mechanical pass as the CI gate risked a regression in a real metrics pipeline without dedicated design review. Recommend routing it through a dedicated design/build lane (e.g.
design_to_planor a scoped worktree with adversarial review) rather than folding it into this gate PR.RED → GREEN evidence
git show HEAD~1:.../handler_chain_verify.py) viascan_handler_filereproduces the ticket's own failure criterion — 1 finding (_TRANSITIONS, module-level dict).uv run python scripts/validate.py orchestrator_reducer_state --verbosereports0 finding(s) across 26 target node dir(s); both pre-commit (changed-node) and CI (full-repo) modes verified locally.pytest tests/unit/(pre-push gate) passes (21846 passed, 36 skipped) after re-running under lower system load — two performance/stress microbenchmarks (test_semver_cache_performance,test_stress_semver_cache_performance, both explicitly documented as "too sensitive to system noise") flaked once under extreme concurrent-session CPU contention (load avg 107) and passed cleanly in isolation and on the full re-run; unrelated to this diff.mypy,ruff format --check,ruff check, and the repo's non-optional-union ratchet (scripts/validate.py unions) all pass (154/154, at the ceiling — fixed a first-draft regression that introduced one non-optional union).Test plan
uv run pytest tests/unit/nodes/node_architecture_validator/test_orchestrator_reducer_state_invariant.py -v— 13 passeduv run pytest tests/unit/nodes/node_architecture_validator/— 253 passeduv run python scripts/validate.py orchestrator_reducer_state --verbose— PASS, 0 findingsuv run mypy src/omnibase_infra/nodes/node_architecture_validator/validators/scanner_orchestrator_reducer_state_invariant.py— no issuespre-commit run onex-orchestrator-reducer-state-invariant— Passedpytest tests/unit/pre-push gate — 21846 passed, 36 skippedgh pr checks) — verify after pushEvidence-Ticket: OMN-14222
Evidence-Source: OCC#4594
Evidence-Commit: f6061f31dc79d3e591136be6816864e7f41e672d