feat(config): reject infeasible parameter sets before running (finding #4) by stranske · Pull Request #5357 · stranske/Trend_Model_Project

stranske · 2026-05-31T02:20:17Z

Policy

A simulation must not run on internally contradictory or under-specified parameters. The only escape for a capacity shortfall is an explicit constraints.cash_weight — never a silent residual. (e.g. a long-only, fully-invested book of 20 funds at a 3% cap is rejected unless cash is declared.)

Checks added (in `config/validation.py`, run during validation)

max_weight < min_weight
multi_period.min_funds > max_funds
vol_adjust.floor_vol >= target_vol (when targeting enabled)
Capacity: max_weight × N < 1 (can't fill a fully-invested book) and min_weight × N > 1 (can't fit), where N is the configured selection cap (rank top_n / multi_period.max_funds) or the available universe. Skipped when undeterminable or when explicit cash is set.

Entry points gated

CLI — already calls validate_config, so the new checks fire there.
Streamlit app — its existing validation only covers a minimal payload subset, so this adds _assert_config_feasible on the full built config in _execute_analysis (via a shared collect_feasibility_errors helper). Infeasible configs surface a clear error and do not run.
api.run_simulation (low-level) stays ungated, so legitimate small-universe programmatic/test use is unaffected (avoids the blast radius of an unconditional runtime guard).

Context

Builds on #5356 (the loader fix) — constraints now actually reach the engine, so validating them is meaningful.

Verification

New tests/test_config_feasibility.py: infeasible cap rejected, explicit-cash escape, contradictions caught, demo stays valid.
tests/app (284) and the existing semantic-validation suite pass with the change; no committed config/*.yml is newly rejected.

🤖 Generated with Claude Code

…#4) A simulation must not run on internally contradictory or under-specified parameters. Adds a general feasibility gate in config validation: - max_weight < min_weight - multi_period.min_funds > max_funds - vol_adjust.floor_vol >= target_vol (when targeting enabled) - long-only, fully-invested capacity: max_weight * N < 1 (can't fill the book) and min_weight * N > 1 (can't fit), where N is the configured selection cap (rank top_n / multi_period.max_funds) or the available universe. The ONLY escape for a capacity shortfall is an EXPLICIT constraints.cash_weight -- never a silent residual. (e.g. 20 funds at a 3% cap is rejected unless cash is declared.) Gates both human-facing entry points: the CLI (already calls validate_config) and the Streamlit app (new _assert_config_feasible on the full built config in _execute_analysis, via the shared collect_feasibility_errors helper). The low-level api.run_simulation stays ungated so legitimate small-universe programmatic use is unaffected. Builds on the #1 loader fix (PR #5356): constraints now actually reach the engine, so validating them is meaningful. Adds tests/test_config_feasibility.py. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0b94fa9799

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-05-31T02:22:35Z

+            top_n = _coerce_pos_int(rank_cfg.get("n"))
+            if top_n is not None:
+                return top_n


Use the capped holding count for capacity checks

When rank.n is larger than a configured holdings cap, this returns top_n before considering max_funds, but the multi-period engine reads constraints.max_funds/multi_period.max_funds and seeds with min(max_funds, target_n) (src/trend_analysis/multi_period/engine.py:1531 and :2499). For example, rank.n=20, max_funds=5, and max_weight=0.10 passes validation as 20 * .10 >= 1, but the run can only hold 5 funds, so the fully invested book has only 50% capacity and still reaches the downstream constraint failure this gate is meant to prevent.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-05-31T02:22:35Z

+    # 4 & 5. Capacity vs weight bounds for a long-only, fully-invested book.
+    # Only enforced when no explicit cash absorbs the slack.
+    if not long_only or explicit_cash or (max_w is None and min_w is None):
+        return


Validate cash weight covers the capacity gap

This skips all capacity checks whenever any positive cash_weight is present, even if the cash slice is too small to make the remaining risky allocation feasible. For example, with 5 funds, max_weight=0.10, and cash_weight=0.10, the non-cash book still needs to allocate 90% into 50% of capacity; the optimizer later rejects this (_apply_cash_weight checks (1 - cash_weight) / len(non_cash) > max_weight in src/trend_analysis/engine/optimizer.py:205-209), so CLI/Streamlit validation still allows a run that the new feasibility gate is intended to reject before execution.

Useful? React with 👍 / 👎.

Copilot

Pull request overview

Adds a parameter-feasibility gate so simulations fail fast on internally contradictory or under-specified configs. The new checks live in the central config/validation.py (so CLI users get them automatically) and are also wired into the Streamlit _execute_analysis path via a shared collect_feasibility_errors helper, while the low-level api.run_simulation stays ungated to preserve small-universe programmatic use.

Changes:

New _check_portfolio_feasibility in config/validation.py covering max_weight < min_weight, multi_period.min_funds > max_funds, vol_adjust.floor_vol >= target_vol, and max_weight*N < 1 / min_weight*N > 1 capacity for long-only fully-invested books (skipped when explicit constraints.cash_weight is set).
Streamlit _execute_analysis now calls _assert_config_feasible on the built config and raises a ValueError listing every violation before the run starts.
New tests/test_config_feasibility.py covers the rejection paths, the explicit-cash escape, and ensures config/demo.yml stays valid.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File	Description
src/trend_analysis/config/validation.py	Adds feasibility helpers (`collect_feasibility_errors`, `_effective_holding_count`, `_check_portfolio_feasibility`) and wires them into `_run_portfolio_validation`.
streamlit_app/components/analysis_runner.py	Builds a portfolio/multi_period/vol_adjust/data view of the full config and runs the feasibility gate before invoking `run_simulation`.
tests/test_config_feasibility.py	New tests for feasibility rejection paths, explicit-cash escape, and demo-config validity.

+    # 2. multi_period.min_funds must not exceed max_funds.
+    multi_period = config.get("multi_period")
+    if isinstance(multi_period, Mapping):
+        min_funds = _coerce_pos_int(multi_period.get("min_funds"))
+        max_funds = _coerce_pos_int(multi_period.get("max_funds"))
+        if min_funds is not None and max_funds is not None and min_funds > max_funds:
+            _append_issue(
+                errors,
+                ValidationError(
+                    path="multi_period.min_funds",
+                    message="min_funds exceeds max_funds.",
+                    expected=f"<= max_funds ({max_funds})",
+                    actual=min_funds,
+                    suggestion="Set min_funds <= max_funds.",
+                ),
+            )


+def _coerce_pos_int(value: Any) -> int | None:
+    if isinstance(value, bool) or not isinstance(value, int):
+        return None
+    return value if value > 0 else None


+    }
+    errors = collect_feasibility_errors(config_view)
+    if errors:
+        detail = "\n".join(f"- {e.message} {e.suggestion}" for e in errors)


Copilot AI review requested due to automatic review settings May 31, 2026 02:20

Copilot started reviewing on behalf of stranske May 31, 2026 02:20 View session

chatgpt-codex-connector Bot reviewed May 31, 2026

View reviewed changes

Copilot AI reviewed May 31, 2026

View reviewed changes

stranske merged commit 10758bf into phase-3 May 31, 2026
28 checks passed

stranske deleted the fix/config-feasibility-validation branch May 31, 2026 02:34

stranske mentioned this pull request Jun 3, 2026

T3 — config/bridge.py::validate_payload only runs Tier-1 validation #5435

Closed

6 tasks

Navigation Menu

feat(config): reject infeasible parameter sets before running (finding #4) - #5357

feat(config): reject infeasible parameter sets before running (finding #4)#5357
stranske merged 1 commit into
phase-3from
fix/config-feasibility-validation

stranske commented May 31, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot May 31, 2026

Uh oh!

chatgpt-codex-connector Bot May 31, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

stranske commented May 31, 2026

Policy

Checks added (in config/validation.py, run during validation)

Entry points gated

Context

Verification

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot May 31, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot May 31, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Checks added (in `config/validation.py`, run during validation)