[jonahgabriel]

OMN-13645 — Context-ROI terminal-topic flip: project failed roi-runs (un-wedge the experiment battery)

Track C, Deliverable 1. Deliverable 2 is split out as a documented, blocked follow-up (see below).

Problem

node_context_roi_runner declared both terminals in contract.yaml publish_topics — onex.evt.omnimarket.context-roi-run-completed.v1 and onex.evt.omnimarket.context-roi-run-failed.v1 — but _emit_result (handler line ~900) only ever published to the completed topic, so the failed terminal was declared-but-dead. And node_projection_context_roi subscribed only to the completed terminal. Net effect: a fully-failed roi-run was never materialised into context_roi_scores, and the N-arm experiment battery wedged with zero usable rows for failed runs — the never-implemented fix that closed the experiment epic while broken.

Change (Deliverable 1)

• Runner (handler_context_roi_runner.py _emit_result): route a whole-run failure (failed_trials == total_trials, or zero cells) to the FAILED terminal; a run with at least one usable row stays on the COMPLETED terminal. Both terminals carry the same ModelContextRoiRunResult payload. This makes the declared-but-dead failed topic real (producer side).
• Projection (node_projection_context_roi/contract.yaml): subscribe to both terminals (consumer side). The projection handler is terminal-agnostic by construction — the failure outcome is already encoded on each row (final_success / failure_stage), so a failed run materialises rows with final_success=False through the exact same project() path. No fake per-topic branch. Mirrors node_projection_delegation, which already folds both the completed and failed delegation/generation terminals through one operation_match handler.

Canonical compliance

CONTRACT + NODE + HANDLER only. Topics stay contract-sourced (no hardcoded literals). No Plugin*, no new REST, event-bus is the transport. No hardcoded absolute paths; strongly-typed; no defensive defaults / silent fallbacks. Dev lane only — no prod/stability/judge touch.

Deliverable 2 — feedback publisher: BLOCKED follow-up (honest diagnosis)

The brief also asked to build the missing publisher for onex.evt.omnimarket.context-roi-runtime-row-scored.v1 (consumed by node_capsule_effectiveness_feedback_reducer). This cannot be built canonically today without fabricating data:

• The consumer's input model ModelScoredRuntimeRow requires per-capsule identity — factor: EnumContextFactor, content, source_artifact, source_commit, validity_scope (all min_length>=1, used to derive the capsule_hash key).
• The ROI scores (context_roi_scores / ModelAttemptReductionRow) are per-(task × arm × trial) aggregates over a context pack of multiple capsules. They carry context_factor_subset (a label like golden_exemplar), context_pack_hash, outcome, and routing_source — but no single-capsule content/source provenance and no single EnumContextFactor.
• There is no materialised pack→capsule membership keyed by context_pack_hash (the pack is built transiently by node_context_pack_builder_compute; ModelContextPackArtifact carries source_artifact_hash/source_contract_hash, not the capsule_store identity fields). The judge verdict (A2) is likewise not materialised onto the ROI rows.

Producing a valid ModelScoredRuntimeRow from arm-level ROI scores would require fabricating capsule provenance, which the doctrine forbids (no defensive/invented defaults). D2 is therefore blocked on a pack→capsule materialisation seam that overlaps the deferred OMN-13647 production context-pack enrichment workstream. Filed as a follow-up rather than shipping AI-slop.

dod_evidence

DoD contract: docs/work-tracking/contracts/OMN-13645.yaml.

• uv run pytest tests/test_golden_chain_projection_context_roi.py tests/nodes/node_context_roi_runner/test_handler_context_roi_runner.py -q → 55 passed. Proves: a fully-failed run publishes on the failed terminal (and NOT completed); the projection materialises a failed run's rows (final_success=False, failure_stage=generation); handle() projects a failed-terminal envelope; the contract subscribes to both terminals.
• uv run pytest tests/ -q -m 'not kafka' (no -k) → 9821 passed, 48 skipped, 9 deselected.
• uv run mypy src/omnimarket/nodes/node_context_roi_runner/handlers/handler_context_roi_runner.py src/omnimarket/nodes/node_projection_context_roi/handlers/handler_projection_context_roi.py --strict → Success: no issues found.
• uv run python -m omnimarket.nodes.node_runtime_sweep --import-check → clean (334 entry points).
• ruff format + ruff check --fix clean; pre-commit run clean on all changed files.

Deploy

No runtime deploy in this PR — handler-logic + contract-topic change. The serialized dev redeploy + live full-cycle proof (failed roi-run → projection row in context_roi_scores) is run separately by the orchestrator (Redeploy #2). Dev local/cheap_cloud tiers are currently backend-exhausted, so the wiring is proven offline (publish-routing + failed-terminal projection) independent of a fully-green generation.

---

Evidence-Ticket: OMN-13645

Summary by CodeRabbit

• New Features

  • Context-ROI runs now publish to the appropriate terminal topic based on outcome, so fully failed runs are handled separately from successful runs.
  • Failed context-ROI runs are now projected into score records as well, keeping downstream views complete.

• Bug Fixes

  • Fixed an issue where fully failed runs could stop short of projection and never appear in results.
  • Updated coverage to verify both completed and failed run paths end-to-end.

Evidence-Source: OCC#3190

[coderabbitai]

📝 Walkthrough

Walkthrough

Context-ROI run results now route to failed or completed terminal topics based on run outcome. The projection contract subscribes to both terminals, and the runner and projection tests cover failed-run materialization and terminal routing.

Changes

Context-ROI terminal-topic flip

Layer / File(s)	Summary
Contract and projection wiring docs/work-tracking/contracts/OMN-13645.yaml, src/omnimarket/nodes/node_projection_context_roi/contract.yaml, src/omnimarket/nodes/node_projection_context_roi/handlers/handler_projection_context_roi.py	OMN-13645 records the terminal routing change, the blocked follow-up, and the evidence checks; the projection contract and handler docs now describe both runner terminals and the failed-terminal subscription.
Runner terminal routing src/omnimarket/nodes/node_context_roi_runner/handlers/handler_context_roi_runner.py	_emit_result selects the failed terminal for zero-trial or fully failed runs and the completed terminal for runs with usable rows.
Runner terminal tests tests/nodes/node_context_roi_runner/test_handler_context_roi_runner.py	The runner tests assert that successful runs do not publish the failed terminal and that a fully failed run publishes only the failed terminal.
Projection failed-terminal tests tests/test_golden_chain_projection_context_roi.py	The golden-chain tests add the failed-terminal topic constant, verify failed-run rows project with failure metadata, and assert the contract wiring subscribes to the failed terminal topic.

Sequence Diagram(s)

sequenceDiagram
  participant HandlerContextRoiRunner
  participant event_bus
  participant HandlerProjectionContextRoi
  participant context_roi_scores
  alt fully failed run
    HandlerContextRoiRunner->>event_bus: publish context-roi-run-failed
  else usable run
    HandlerContextRoiRunner->>event_bus: publish context-roi-run-completed
  end
  event_bus->>HandlerProjectionContextRoi: deliver subscribed terminal event
  HandlerProjectionContextRoi->>context_roi_scores: upsert rows by correlation_id

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

A bunny hopped by the terminal glen,
and leapt to failed or completed again.
🐇 Rows danced in context_roi_scores bright,
with correlation IDs twinkling at night.
Hoppity hop—two paths now hum in tune.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 30.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: activating the failed terminal and projecting failed context-ROI runs.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch jonah/omn-13645-feedback-flywheel-terminal-topic-flip

---

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

⚠️ Architectural Review — WARNINGS

Errors: 0
Warnings: 1

What this checks

Rule	Description
ARCH-TOPIC-001	No hardcoded Kafka topic strings in handler code
ARCH-DI-001	No event_bus=None bypass
ARCH-DI-002	No direct Handler instantiation outside workflow_runner/adapters
ARCH-DI-003	No reinvented DI containers
ARCH-TOPIC-002	contract.yaml topics follow `onex.{cmd

---

ARCH-DI-002: Direct Handler instantiation (warning)\n\n\nsrc/omnimarket/nodes/node_context_roi_runner/handlers/handler_context_roi_runner.py:285: result = HandlerContextPackBuilder().handle(request)\n\n\nHandler classes should be wired only in workflow_runner.py or adapter modules. Direct instantiation elsewhere bypasses the contract system.\n\n

Powered by node_pr_review_bot

[github-actions]

✅ PR Review Bot — CLEAN

Findings: 0
Run ID: ``

Required gate: This check blocks merge when verdict is blocking_issue.

---

Verdict semantics

Verdict	Meaning
clean	No blocking findings
risks_noted	Findings noted; threads passed judge verification
blocking_issue	MAJOR/CRITICAL findings not resolved — do not merge

Powered by node_pr_review_bot — Qwen3-Coder-30B reviewer + DeepSeek-R1 judge (OMN-7963)

[github-actions]

⚠️ Hostile Reviewer — DEGRADED (informational)

Blocking findings (critical/error): 0
Total findings: 0
Models succeeded: none

Note: All reviewer models failed or were unavailable. Degraded results are informational during the pilot phase (OMN-8524) and do not block merge. Error: cli_review exited non-zero (all models failed or binary error)

---

Gate semantics (pilot phase)

Verdict	Meaning	Blocks merge?
passed	No critical/error findings	No
blocked	CRITICAL or ERROR findings found	Yes
degraded	All models unavailable (infra)	No (pilot)

Powered by omniintelligence.review_pairing.cli_review — node-based adversarial review via HandlerLlmCliSubprocess (OMN-8524)