refactor: add campaign outcome finalization seam - #26
Conversation
Adds a pure campaign_outcome package for deriving explicit campaign outcome truth from runner-fed evidence. Final review now consumes a FinalReviewReadModel projected from CampaignOutcome instead of treating report_ok as campaign success. report_ok remains report-generation evidence only. Wires runner finalization through evaluate_campaign_outcome and project_final_review, adds outcome-based exit-code handling, and preserves measurement truth separately from report/artifact success. Adds focused evaluator/projection tests plus runner hardening regressions for report_ok false-success prevention and report-failure preservation.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
WalkthroughAdds a campaign-outcome seam: frozen contracts, a pure evaluator that derives authoritative outcomes from runner-provided evidence, a projector that builds a final read-model, and runner + UI integration that aggregates DB evidence, evaluates/projects outcomes, renders the post-run review, and centralizes exit semantics. ChangesCampaign Outcome Evaluation Pipeline
Sequence DiagramsequenceDiagram
participant Runner
participant Evaluator
participant Projector
participant UI
Runner->>Runner: Aggregate campaign evidence (DB)
Runner->>Runner: Build CampaignOutcomeInputs (evidence, flags, hints)
Runner->>Evaluator: evaluate_campaign_outcome(inputs)
activate Evaluator
Evaluator->>Evaluator: Early-abort & fatal checks
Evaluator->>Evaluator: Derive measurement & post-run verdicts
Evaluator->>Evaluator: Synthesize outcome_kind, failure metadata, authority flags
Evaluator-->>Runner: CampaignOutcome
deactivate Evaluator
Runner->>Projector: project_final_review(outcome, metrics, runner_cause)
activate Projector
Projector->>Projector: Select headline, show_next_actions, diagnostics
Projector->>Projector: Synthesize failure_cause/remediation and artifact mode
Projector-->>Runner: FinalReviewReadModel
deactivate Projector
Runner->>UI: render_post_run_review_from_read_model(campaign_id, read_model)
activate UI
UI->>UI: Render headline, report/meta, failure/blocker, artifacts, next-actions, diagnostics
deactivate UI
Runner->>Runner: Exit with code based on allows_success_style_review
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 👉 Get your free trial and get 200 agent minutes per Slack user (a $50 value). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Stale comment
Coverage automation added a follow-up test PR: #27
Covered regression risks:
- pre-measurement startup aborts for telemetry readiness and backend execution policy blocks
- fatal measurement exceptions preserving failed measurement semantics
- runner DB evidence aggregation, including OOM/skipped-OOM/degraded config counts, invalid cycle counts, and ignoring successful requests from invalid cycles
Updated test files:
test_campaign_outcome_slice1.pytest_runner_campaign_outcome_hardening.pyValidation:
.venv/bin/python -m ruff check test_campaign_outcome_slice1.py test_runner_campaign_outcome_hardening.pyQUANTMAP_LAB_ROOT=/tmp/quantmap-lab .venv/bin/python -m pytest test_campaign_outcome_slice1.py test_runner_campaign_outcome_hardening.pyNote: the first pytest attempt without
QUANTMAP_LAB_ROOTfailed at import-time due to the repo's environment requirement; rerun with a temp lab root passed.Sent by Cursor Automation: Add test coverage
There was a problem hiding this comment.
Pull request overview
This PR introduces a new “campaign outcome finalization seam” by adding a pure src/campaign_outcome package that derives an explicit CampaignOutcome from runner-fed evidence, projects it into a final-review read model, and updates the runner/UI to consume that read model instead of using report_ok as a proxy for campaign success.
Changes:
- Add
src/campaign_outcome(contracts + pure evaluator + projection to a UI-facing read model). - Update
src/runner.pyto evaluate outcome + project a final-review read model and drive exit behavior fromCampaignOutcome. - Update UI and tests to render/validate the new final-review read model path.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/campaign_outcome/init.py | Exposes the package-root API surface for outcome evaluation/projection. |
| src/campaign_outcome/contracts.py | Defines frozen dataclass contracts and enums for runner-fed inputs and evaluator outputs. |
| src/campaign_outcome/evaluate.py | Implements pure outcome evaluation logic from CampaignOutcomeInputs. |
| src/campaign_outcome/projection.py | Projects CampaignOutcome (+ optional metrics/runner strings) into FinalReviewReadModel. |
| src/runner.py | Feeds evidence into the evaluator, renders final review from the read model, and gates exit code on the evaluated outcome. |
| src/ui.py | Adds render_post_run_review_from_read_model to render final review via the new read model. |
| test_campaign_outcome_slice1.py | Unit tests for pure evaluation/projection behavior and package-root exports. |
| test_runner_campaign_outcome_hardening.py | Runner-level integration tests hardening exit/final-review behavior against partial/failed evidence. |
| test_cli_ux_yolo_review.py | Updates YOLO review test harness to route through the new read-model renderer. |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/runner.py (1)
3062-3065:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSplit analysis failures from report-generation failures here.
If
analysis_okis alreadyTrue, this exception came from report/export work, not scoring. Reusing “Post-campaign analysis failed” and “quantmap rescore” here gives operators the wrong diagnosis and recovery path, and that generic string is then fed into the final-review projection.Suggested fix
except Exception as exc: logger.error("Post-campaign analysis failed: %s", exc, exc_info=True) - failure_cause = "Post-campaign analysis failed." - failure_remediation = "Run 'quantmap rescore' to retry analysis." + if analysis_ok: + failure_cause = "Primary report generation failed." + failure_remediation = None + else: + failure_cause = "Post-campaign analysis failed." + failure_remediation = "Run 'quantmap rescore' to retry analysis." with get_connection(_eff_db_path) as _status_conn:As per coding guidelines, "Prioritize correctness, determinism, and benchmark integrity. Flag behavior changes that affect scoring, governance, reporting, telemetry, methodology snapshots, or trust semantics."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/runner.py` around lines 3062 - 3065, The except block currently always sets failure_cause and failure_remediation for analysis failures; update it to distinguish analysis vs report/export failures by checking the analysis_ok flag: keep the existing "Post-campaign analysis failed." / "Run 'quantmap rescore' to retry analysis." messages when analysis_ok is False (meaning scoring/analysis failed), but when analysis_ok is True set failure_cause to something like "Report/export generation failed." and failure_remediation to a report-specific action (e.g., "Inspect report/export logs and retry report generation" or the appropriate command), and still call logger.error(..., exc_info=True) to include the exception details; modify the except handling around logger.error, failure_cause, and failure_remediation to implement this branch using the analysis_ok variable.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/campaign_outcome/evaluate.py`:
- Around line 86-97: The current allows_rec logic grants recommendation
authority for MeasurementPhaseVerdict.PARTIAL; change the verdict check in
src/campaign_outcome/evaluate.py so only MeasurementPhaseVerdict.SUCCEEDED
qualifies (remove PARTIAL from the tuple or replace the membership test with an
equality check to SUCCEEDED). Keep the rest of the gating conditions
(ev.has_any_success_request, inputs.scoring_completed, inputs.passing_count > 0,
inputs.winner_config_id is not None, inputs.report_ok is True, and the various
negative flags) unchanged so that allows_rec only becomes true for a SUCCEEDED
measurement.
In `@src/campaign_outcome/projection.py`:
- Around line 35-36: The code sets failure_cause using "runner_failure_cause or
outcome.failure_detail" which lets a generic runner string override the
authoritative evaluator message; change the assignment to prefer the evaluator
detail first by using "outcome.failure_detail or runner_failure_cause" so the
evaluator's message (outcome.failure_detail) is used when present and the runner
value (runner_failure_cause) is only a fallback; update the assignment for the
failure_cause variable in projection.py accordingly (leave failure_remediation
as-is).
In `@src/ui.py`:
- Around line 588-735: render_post_run_review_from_read_model duplicates the
config-summary, blocker, artifact, next-action, and diagnostics rendering
already implemented in render_post_run_review; extract those shared branches
into one or more helper functions and have both entrypoints call them to avoid
drift. Specifically, create a helper (e.g., _render_post_run_common or similar)
that accepts the Console, FinalReviewReadModel(or mapped PostRunReviewMetrics),
metrics, campaign_id, artifacts, and diagnostics_path and moves the repeated
logic that references PostRunReviewMetrics, _MODE_LABELS/_format_elapsed,
failure_cause/failure_remediation/CampaignOutcomeKind, render_artifact_block,
and print_next_actions into that helper; then update
render_post_run_review_from_read_model and render_post_run_review to call the
new helper after mapping metrics (or adapt the legacy entrypoint to build a
FinalReviewReadModel and delegate) so presentation logic is centralized and
deterministic.
---
Outside diff comments:
In `@src/runner.py`:
- Around line 3062-3065: The except block currently always sets failure_cause
and failure_remediation for analysis failures; update it to distinguish analysis
vs report/export failures by checking the analysis_ok flag: keep the existing
"Post-campaign analysis failed." / "Run 'quantmap rescore' to retry analysis."
messages when analysis_ok is False (meaning scoring/analysis failed), but when
analysis_ok is True set failure_cause to something like "Report/export
generation failed." and failure_remediation to a report-specific action (e.g.,
"Inspect report/export logs and retry report generation" or the appropriate
command), and still call logger.error(..., exc_info=True) to include the
exception details; modify the except handling around logger.error,
failure_cause, and failure_remediation to implement this branch using the
analysis_ok variable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: cc0f17db-f2d0-4088-9909-8dfaccc3642d
⛔ Files ignored due to path filters (3)
test_campaign_outcome_slice1.pyis excluded by none and included by nonetest_cli_ux_yolo_review.pyis excluded by none and included by nonetest_runner_campaign_outcome_hardening.pyis excluded by none and included by none
📒 Files selected for processing (6)
src/campaign_outcome/__init__.pysrc/campaign_outcome/contracts.pysrc/campaign_outcome/evaluate.pysrc/campaign_outcome/projection.pysrc/runner.pysrc/ui.py
There was a problem hiding this comment.
Stale comment
Coverage automation added a follow-up test/fix PR: #28
Covered regression risks:
- stale
report_ok=Trueno longer masks explicitreport_status="failed"report_status="skipped"no longer gets success-style review or recommendation authority- measurement truth remains valid while post-run/reporting failure blocks final success semantics
Updated files:
test_campaign_outcome_slice1.pysrc/campaign_outcome/evaluate.pyValidation:
.venv/bin/python -m ruff check src/campaign_outcome/evaluate.py test_campaign_outcome_slice1.pyQUANTMAP_LAB_ROOT=/tmp/quantmap-lab .venv/bin/python -m pytest -q test_campaign_outcome_slice1.pyQUANTMAP_LAB_ROOT=/tmp/quantmap-lab .venv/bin/python -m pytest -q test_runner_campaign_outcome_hardening.pygit diff --checkNote: the local checkout did not include a working
.venv; I installedpython3.12-venv, recreated.venv, and installed.[dev]before running validation.Sent by Cursor Automation: Add test coverage
Normalize explicit report status before report_ok, tighten recommendation authority to fully successful measurement evidence, preserve evaluator failure detail in projection, distinguish report/export failures from analysis failures, and add adversarial coverage for conflicting post-run truth.
Extract shared private post-run rendering helpers so legacy and read-model final-review entrypoints share presentation logic without changing outcome semantics, public renderer signatures, or UI behavior.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/ui.py (1)
512-552:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftReduce complexity of
_render_post_run_config_summary_and_metato satisfy CI.Line 512 exceeds the configured cognitive complexity limit (19 > 15). Extract config-summary composition and meta-line composition into small helpers to keep behavior unchanged and clear the failing quality gate.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/ui.py` around lines 512 - 552, The function _render_post_run_config_summary_and_meta is too complex; extract the config-summary assembly and the meta-parts assembly into two small helpers (for example, _compose_config_summary(metrics) -> tuple[bool, list[str]] or list[str] and _compose_meta_parts(metrics) -> list[str]) that encapsulate the branches that build _parts/_has_config_summary and _meta_parts respectively, then replace the inlined blocks in _render_post_run_config_summary_and_meta with calls to those helpers and keep the con.print calls and semantics unchanged (including the winner_config_id/winner_tg logic and the fallback empty line when _has_config_summary is true but no meta parts). Ensure helper names reference the original metrics parameter and preserve formatting tokens like "[green]" "[dim]" and the call to _format_elapsed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/campaign_outcome/evaluate.py`:
- Around line 220-319: _synthesize_outcome is too complex; split its large
decision tree into small ordered rule helpers to reduce cognitive complexity
while preserving current precedence and return values. Create helper functions
like _measurement_gate(inputs, measurement, measurement_failure_domain),
_scoring_gate(inputs, post_run), and _post_run_report_gate(inputs, post_run)
that each encapsulate the contiguous if-blocks currently handling measurement
verdicts (MeasurementPhaseVerdict.*), scoring completion and winner checks
(scoring_completed, passing_count, winner_config_id), and post-run/report
verdicts (PostRunVerdict.*) respectively; have _synthesize_outcome call these
helpers in the same order as the original logic and return early when a helper
returns a non-None outcome tuple (CampaignOutcomeKind, FailureDomain|None,
str|None), preserving messages and uses of symbols like CampaignOutcomeKind,
FailureDomain, MeasurementPhaseVerdict, PostRunVerdict, inputs.evidence,
inputs.last_backend_failure_reason, and inputs.campaign_db_status so behavior
and precedence remain identical.
In `@src/runner.py`:
- Around line 2723-2733: run_campaign currently swallows KeyboardInterrupt by
doing a bare return, which yields a zero exit in the CLI path; instead of
returning, set the exit state/details (campaign_exit_state,
campaign_exit_detail) and propagate a non-success outcome back to the caller by
returning or raising the canonical exit object used by the runner (e.g.,
construct and return the CampaignOutcome with state "INTERRUPTED" and the detail
string, or raise SystemExit with a non‑zero code), so the CLI finalization path
sees the interrupted outcome; update the KeyboardInterrupt handler in
run_campaign (replace the bare return) to emit the INTERRUPTED outcome via the
project's standard finalization mechanism.
---
Outside diff comments:
In `@src/ui.py`:
- Around line 512-552: The function _render_post_run_config_summary_and_meta is
too complex; extract the config-summary assembly and the meta-parts assembly
into two small helpers (for example, _compose_config_summary(metrics) ->
tuple[bool, list[str]] or list[str] and _compose_meta_parts(metrics) ->
list[str]) that encapsulate the branches that build _parts/_has_config_summary
and _meta_parts respectively, then replace the inlined blocks in
_render_post_run_config_summary_and_meta with calls to those helpers and keep
the con.print calls and semantics unchanged (including the
winner_config_id/winner_tg logic and the fallback empty line when
_has_config_summary is true but no meta parts). Ensure helper names reference
the original metrics parameter and preserve formatting tokens like "[green]"
"[dim]" and the call to _format_elapsed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: fcf64266-555a-4715-8314-48a41ba9ef90
⛔ Files ignored due to path filters (3)
test_campaign_outcome_slice1.pyis excluded by none and included by nonetest_cli_ux_yolo_review.pyis excluded by none and included by nonetest_runner_campaign_outcome_hardening.pyis excluded by none and included by none
📒 Files selected for processing (5)
src/campaign_outcome/contracts.pysrc/campaign_outcome/evaluate.pysrc/campaign_outcome/projection.pysrc/runner.pysrc/ui.py
Preserve backend-startup failure domains during no-success finalization, exit interrupted campaigns with code 130, and align artifact test fixtures with canonical artifact constants.
Split campaign outcome synthesis into ordered private gates, reduce post-run review helper complexity, and remove duplicate metrics DTO drift by reusing the final-review metrics snapshot contract.
Manually port useful Cursor Bugbot regression coverage for pre-measurement aborts, fatal measurement failures, and campaign evidence aggregation while leaving stale or redundant bot-generated behavior out of the branch.
Add targeted docstrings for the campaign outcome contracts, evaluator, projector, runner evidence aggregation, and post-run UI rendering to clarify ownership of outcome truth and presentation boundaries.
There was a problem hiding this comment.
Stale comment
Coverage automation added a follow-up test PR: #30
Covered regression risk:
- structured
report_status="complete"now explicitly overrides stalereport_ok=False, matching the new structured-status truth lane and completing coverage alongside failed/skipped/partial report-status branchesUpdated files:
test_campaign_outcome_slice1.pysrc/campaign_outcome/evaluate.pyWhy this materially reduces regression risk:
- the campaign outcome seam drives user-facing success review and process exit semantics, so raw boolean precedence over structured report state could incorrectly downgrade otherwise valid completed campaigns
- the new test locks the intended precedence at the pure evaluator layer, where it is deterministic and independent of runner/UI side effects
Validation:
.venv/bin/python -m ruff check src/campaign_outcome/evaluate.py test_campaign_outcome_slice1.pyQUANTMAP_LAB_ROOT=/tmp/quantmap-lab .venv/bin/python -m pytest -q test_campaign_outcome_slice1.py(24 passed)git diff --checkSent by Cursor Automation: Add test coverage
Carry synthesized abort reasons through campaign outcome evaluation, treat explicit complete report status as authoritative, and route KeyboardInterrupt through evaluator-backed final review before exiting 130.
Use a single module import style for campaign outcome evaluator tests to satisfy CodeQL import hygiene while preserving behavior.
There was a problem hiding this comment.
Stale comment
Coverage automation added follow-up PR: #31
Covered regression risks:
- OOM boundary sweeps with valid completed measurement evidence and a winner are no longer downgraded to partial solely because OOM configs produced invalid cycles.
- Interrupted campaigns cannot fall through to the normal completion path if interrupt finalization is suppressed or returns unexpectedly.
Updated files:
test_campaign_outcome_slice1.pytest_runner_campaign_outcome_hardening.pysrc/campaign_outcome/evaluate.pysrc/runner.pyWhy this materially reduces regression risk:
- campaign outcome finalization controls user-facing success review, recommendation authority, and exit semantics, so misclassifying OOM boundary discovery or interrupted runs can mislead automation and operators.
Validation:
python3 -m ruff check src/campaign_outcome/evaluate.py src/runner.py test_campaign_outcome_slice1.py test_runner_campaign_outcome_hardening.pyQUANTMAP_LAB_ROOT=/tmp/quantmap-lab python3 -m pytest -q test_campaign_outcome_slice1.py test_runner_campaign_outcome_hardening.pyQUANTMAP_LAB_ROOT=/tmp/quantmap-lab python3 -m pytest test_campaign_outcome_slice1.py test_runner_campaign_outcome_hardening.py --cov=src.campaign_outcome --cov=src.runner --cov-report=term-missinggit diff --checkSent by Cursor Automation: Add test coverage
Guarantee interrupted campaigns exit 130 even if final review rendering fails, keep normal completion unreachable after interruption, narrow boundary-invalid measurement classification to OOM-backed evidence, and remove remaining artifact fixture drift.
Address SonarCloud duplication and code-smell findings by simplifying repeated campaign outcome test setup, replacing flagged constructor/comprehension patterns, and removing an unused UI console parameter without changing runtime behavior.
Treat interrupted and pre-DB aborted campaigns as report-not-attempted, preserve partial artifact warnings without invalidating core-valid campaign results, and add regression coverage for report authority, interrupt copy, and partial-measurement exit behavior.
Document the partial-artifact success-style carve-out, normalize structured report status before legacy report_ok display state, and add regression coverage so partial secondary artifacts cannot contradict primary report truth.
Centralize runner outcome input assembly, tighten OOM-invalid evidence classification, align report_ok tri-state defaults, harden post-run failure rendering, and reduce Sonar-triggering test duplication without changing finalization ownership boundaries.
Distinguish report_ok=None from report failure in campaign outcome finalization so pre-report and unknown report paths do not surface misleading failed-report UI while preserving structured report_status precedence.
Use explicit None coalescing for campaign outcome passing and eliminated counts so real zero counts are not obscured by truthiness-based defaults.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/campaign_outcome/evaluate.py`:
- Around line 217-224: The early-return branch that currently returns
(MeasurementPhaseVerdict.NO_EVIDENCE, FailureDomain.MEASUREMENT_BODY) when
ev.cycles_attempted == 0, ev.configs_completed == 0, and not
ev.has_any_success_request should first check inputs.last_backend_failure_reason
(the runner-provided lbr) and, if present, call the existing helper
_outcome_gate_measurement_failed to produce a measurement-failed verdict that
surfaces that lbr in failure_detail; otherwise keep the existing behavior (if
ev.configs_total == 0 return NOT_STARTED, else return
NO_EVIDENCE/MEASUREMENT_BODY). Ensure you reference ev,
inputs.last_backend_failure_reason, MeasurementPhaseVerdict,
FailureDomain.MEASUREMENT_BODY and _outcome_gate_measurement_failed when making
the change.
In `@src/campaign_outcome/projection.py`:
- Around line 67-70: Normalize whitespace-only failure strings before computing
the fallback: trim outcome.failure_detail (and similarly runner_failure_cause if
appropriate) and treat an all-whitespace result as None so the fallback chain
can pick runner_failure_cause or the abort reason. Update the assignment to
failure_cause (and any upstream uses of runner_failure_cause) to use the
stripped value or None before applying "or runner_failure_cause" and before the
abort check that inspects outcome.abort.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 384ffd9b-e12a-4e72-baea-ae352e340a3a
⛔ Files ignored due to path filters (1)
test_runner_campaign_outcome_hardening.pyis excluded by none and included by none
📒 Files selected for processing (4)
src/campaign_outcome/evaluate.pysrc/campaign_outcome/projection.pysrc/runner.pysrc/ui.py
|
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 783faf1. Configure here.
|






Summary
Completes Campaign Outcome / Finalization Slice 1.
This PR introduces a pure
src/campaign_outcomepackage that derives explicit campaign outcome truth from runner-fed evidence, then projects that truth into a final-review read model.Final review is now driven by:
evaluate_campaign_outcome→project_final_review→render_post_run_review_from_read_modelinstead of using raw
report_okas the success signal.This PR also completes the review hardening that followed the initial slice:
render_post_run_reviewpath was removedFinalReviewReadModelget_console(force_utf8_if_bootstrap=...)compatibility was restored as a no-op keyword parameterWhy
Before this change, final-review success could be inferred from post-run/reporting behavior instead of campaign truth. In practice, a successful primary report could make the end-of-run UX look successful even when measurement evidence, scoring, ranking, or recommendation authority did not justify that outcome.
That violated QuantMap's trust boundary:
observation != interpretation != scoring != ranking != recommendation != presentationThis PR creates a dedicated outcome module so finalization truth is evaluated once, projected once, and then rendered by the UI without reinterpreting success/failure state.
Scope
Scope type: Broad but bounded architectural slice.
In scope:
src/campaign_outcome/src/runner.pysrc/ui.pyFinalReviewReadModelreport_okrenderingget_consolecompatibility restorationOut of scope:
CampaignOutcometo DBcampaigns.statusArchitecture / trust boundary
This PR separates:
report_oknow means only "primary report generation succeeded." It is still passed into the evaluator as post-run evidence, but it no longer decides campaign success by itself.src/campaign_outcomeis pure and runner-fed. It does not import UI, runner, DB, report/export code, subprocess, filesystem, or SQLite.The runner acts as the imperative shell:
evaluate_campaign_outcomeproject_final_reviewrender_post_run_review_from_read_modelThe UI now renders the read model. It does not infer outcome truth.
Behavior changes
Exit behavior is now stricter.
Previously, a successful primary report could effectively produce success-style finalization. Now, exit
0requires a success-styleCampaignOutcome.A generated report is not enough for success if the campaign has no valid measurement evidence, no rankable result, or otherwise fails the outcome gate.
Measurement truth and report/artifact truth are separate:
0Handled aborts now route through the outcome module:
Unexpected fatal measurement exceptions still fail loud. They now attempt best-effort final-review projection first, then re-raise the original exception so traceback and failure semantics are preserved.
The legacy
render_post_run_reviewpath was removed. Final review now renders only fromFinalReviewReadModel.Risk
Primary risk:
1when the campaign outcome is partial, insufficient, failed, aborted, or otherwise not success-styleCompatibility risk mitigated:
get_console(force_utf8_if_bootstrap=...)was restored as a no-op compatibility parameter so external callers do not receive an unexpectedTypeErrorCrash-path risk mitigated:
Review/maintenance risk mitigated:
report_okUI path was removed after coverage migrationTests
Added:
test_campaign_outcome_slice1.pytest_runner_campaign_outcome_hardening.pyUpdated:
test_cli_ux_post_run_review.pytest_cli_ux_yolo_review.pyCoverage includes:
report_ok=Truecannot mask missing measurement successreport_okUI rendering was removed without dropping final-review behavior coverageget_console(force_utf8_if_bootstrap=True)remains backward-compatibleValidation
Validated locally with:
verify_dev_contract.py --quick— PASSruff checkon touched paths — PASSmypyon touched paths — PASSmypy quantmap.pyduring the slice — PASSchanged_path_verify.py— PASSgit diff --check— OKAdditional focused validation after review/Sonar fixes:
test_runner_campaign_outcome_hardening.py— PASStest_cli_ux_post_run_review.py— PASSruff— PASSmypy— PASSgit diff --check— OKAgent Surface
Notes / follow-ups
Deferred intentionally:
CampaignOutcometo DBcampaigns.statusThe per-cycle invalidation/OOM attribution limitation is known and intentionally deferred. The current implementation uses the evidence available in this slice and avoids making stronger claims than the data supports.
Note
Medium Risk
Changes campaign completion truth/exit-code semantics by introducing a new outcome evaluator and routing abort/failure paths through it, which can affect automation and user-visible final-review behavior. Risk is moderate because it touches the main runner finalization flow but is largely additive and contract-driven.
Overview
Introduces a new
src/campaign_outcomeseam with frozen contracts (CampaignOutcomeInputs,CampaignOutcome,FinalReviewReadModel), a pureevaluate_campaign_outcomedecision function, andproject_final_reviewto map outcome truth into UI-ready presentation fields.Updates
runner.pyto build DB-backed measurement evidence, evaluate outcome truth (including handled aborts, fatal measurement exceptions, and post-run report/scoring states), render the final review viarender_post_run_review_from_read_model, and set process exit code based onCampaignOutcome.allows_success_style_reviewinstead of rawreport_ok.Refactors
ui.pyto remove the legacy binaryrender_post_run_reviewpath and render post-run review purely from theFinalReviewReadModel, including headline/status styling, conditional next-actions, diagnostics messaging, and artifact table visibility.Reviewed by Cursor Bugbot for commit 03f522c. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
New Features
Bug Fixes
Refactor