feat(shadow-account): carry price-condition bounds (RSI, prior return) into extracted rules

[Robin1987China]

Summary

Extracted rules now carry price-condition bounds when entry_rsi14 and prior_5d_return are promoted into clustering (Step 1, #302). The generated SignalEngine uses these bounds for real conditional entry logic instead of blind hold-day replay.

Why

Step 1 (#302) added price features to KMeans clustering but kept them silent in output rules. The generated SignalEngine still replays hold-day cadences blindly — enter every N bars. A rule like "enter when RSI is in 25-45" is far more actionable and backtestable. This PR closes the gap: promoted price features now flow through into rule entry_condition dicts, and the generated engine uses conditional entry logic.

Changes

• agent/src/shadow_account/extractor.py: _cluster_to_rule stores promoted price features in entry_condition as {min: p10, max: p90}; all-NaN features silently skipped.
• agent/src/shadow_account/codegen.py: _rule_to_context flattens optional entry_rsi14/prior_5d_return bounds for the template.
• agent/src/shadow_account/templates/signal_engine.py.j2: SignalEngine._conditional_entry gates entry on real-time RSI and prior-5d-return when bounds present, falls back to time-window-only otherwise.
• agent/tests/test_shadow_account.py: 16 new tests — extraction (5), codegen (4), template behavior (10), security injection (1).

Test Plan

• ruff check extractor.py / codegen.py — 0 errors (template .j2 F821 is pre-existing Jinja2)
• pytest agent/tests/test_shadow_account.py — 62 passed
• pytest --ignore=agent/tests/e2e_backtest --ignore=agent/tests/test_e2e_harness_v2.py — 4404 passed, 1 skipped
• openspec validate --changes --strict — 4 items passed
• pytest agent/tests/test_shadow_codegen_security.py — 1 passed

Checklist

• No changes to protected areas without prior discussion
• No hardcoded values
• Code follows CONTRIBUTING.md guidelines
• Documentation updated (tasks.md T3.4/T5.7 wording corrected per implementation)

Refs: #295

[shadowinlife]

Code Review — 5-Agent Parallel Review

Reviewed by 5 parallel agents: Goal Verification, Code Quality, Security, QA Execution, and Context Mining.

Overall Verdict: ❌ FAILED (2 blocking issues)

#	Review Area	Verdict	Confidence
1	Goal & Constraint Verification	✅ PASS	HIGH
2	Code Quality	❌ FAIL	HIGH
3	Security	✅ PASS	NONE
4	QA Execution	⚠️ INCONCLUSIVE	HIGH
5	Context Mining	❌ FAIL	HIGH

---

🔴 Blocking Issue 1: scanner.py incompatible with new price-condition format

PR #314 stores price conditions as {"min": p10, "max": p90} dicts in entry_condition, but scanner.py (the scan_shadow_signals tool) was not updated:

• _feature_for_condition_key("entry_rsi14") → returns None → _entry_condition_matches returns False → rule NEVER matches in scanner
• _feature_for_condition_key("prior_5d_return") → maps to "momentum" → _compare(value, {"min": -0.05, "max": 0.02}) calls _to_float(dict) → returns None → condition silently skipped

Impact: Users calling scan_shadow_signals on a profile with price-condition rules will get zero or degraded matches with no error message. The backtest path (generated SignalEngine) works correctly, but the real-time scanner is broken for these rules.

Suggested fix:

1. Add "entry_rsi14" mapping in _feature_for_condition_key + RSI computation in scanner's _compute_features
2. Extend _compare to handle {"min": lo, "max": hi} dict conditions (range check: lo <= value <= hi)
3. Add scanner tests for price-condition-bearing rules

---

🟡 Blocking Issue 2: _heuristic_single_rule doesn't pass price_features

In _extract_rules, the multi-cluster path correctly computes and passes promoted_price_features to _cluster_to_rule. But _heuristic_single_rule (the fallback when clustering yields ≤1 cluster) calls _cluster_to_rule without price_features, defaulting to ():

def _heuristic_single_rule(features_df, min_support, llm_translator):
    return _cluster_to_rule(
        cluster_df=features_df, rule_index=1,
        total_profitable=max(len(features_df), min_support),
        llm_translator=llm_translator,
        # price_features NOT passed → defaults to ()
    )

Impact: Single-cluster extraction (small journals, homogeneous trades) will never produce rules with RSI/return bounds, even when price data is available. Users get silently inferior rules.

Suggested fix: Accept and forward price_features in _heuristic_single_rule:

def _heuristic_single_rule(features_df, min_support, llm_translator, *, price_features=()):
    return _cluster_to_rule(
        cluster_df=features_df, rule_index=1,
        total_profitable=max(len(features_df), min_support),
        llm_translator=llm_translator,
        price_features=price_features,
    )

---

Non-blocking Findings

#	Category	Finding	Severity
1	Performance	Template generate() unconditionally computes RSI and prior_return for every code, even when the matched rule has no price bounds	MINOR
2	Maintainability	Feature name tuple ("entry_rsi14", "prior_5d_return") is hard-coded in 3 places (extractor _PRICE_FEATURES, codegen loop, template methods) — consider a single shared constant	MINOR
3	Precision	round(quantile, 2) on prior_5d_return (typical range ±0.0x) loses meaningful precision; consider round(..., 4) for return-type features	NITPICK
4	Security Testing	No adversarial injection tests for the codegen pipeline (e.g., crafted rule_id with shell commands)	MINOR
5	Docs	PR checklist claims "tasks.md T3.4/T5.7 wording corrected" but tasks.md does not exist in the repository	FYI

---

Positive Observations

• Defense-in-depth code generation safety: _literal_safe_value (type whitelist) → _python_literal (repr() + ast.literal_eval roundtrip) → validate_generated (ast.parse + shape check). Three-layer protection against injection — solid.
• Clean data flow: extractor → codegen → template key naming is consistent (entry_rsi14_min/max, prior_5d_return_min/max)
• Robust NaN handling: Three layers of protection (dropna() → len >= 2 → isinstance checks)
• Backward compatibility: Rules without price bounds gracefully skip price checks via rule.get() returning None
• RSI implementation: Correctly mirrors compute_rsi in example_signal_engine.py:13 per [Feature] Trade Journal -> strategy extraction -> backtest bridge (Phase 4c) - planning question #295 design guidance
• Test coverage: 16 new tests — comprehensive coverage of extraction, codegen, template behavior, and security injection

---

Recommended Fix Priority

1. Update scanner.py to handle {"min": lo, "max": hi} condition format and add RSI computation — silent regression
2. Pass price_features to _heuristic_single_rule — one-line fix for behavioral consistency
3. Consider conditional computation in the template (only compute RSI/return when any rule uses them)
4. Add adversarial codegen tests to lock the security contract

[Robin1987China]

Thanks for the careful read — both blocking issues were real, pushed fixes in fbff848.

Blocking 2 (_heuristic_single_rule) — fixed exactly as suggested. It now takes price_features and forwards it: available_price_features on the len < min_support branch and promoted_price_features on the no-rules fallback. Added a test (test_single_cluster_fallback_carries_price_bounds) that forces the single-rule path and asserts the bounds land in entry_condition.

Blocking 1 (scanner.py) — fixed: added _compute_rsi + an entry_rsi14 feature in _compute_features, mapped the rsi key in _feature_for_condition_key, and taught _compare to range-check {"min","max"} dicts. Added 5 scanner tests covering RSI in/out-of-band, the prior_5d_return dict band, and the insufficient-history case.

One correction on the impact though: it was not "rule NEVER matches / zero matches". prior_5d_return already maps to momentum, so checked flips true and the rule still matched — the {min,max} bound was just silently skipped (dict hit _to_float → None → _compare returned True). RSI-only rules fell through to the default momentum+MA heuristic. So the real bug was degraded precision / a scanner-vs-backtest gap, not a hard miss. Either way it is fixed now and the two paths agree.

On finding #5 (tasks.md): that file lives in a local planning dir I keep out of the repo (same as the earlier autopilot PRs), so its absence here is expected, not a missing file.

Non-blocking items — the unconditional RSI compute in the template, the hard-coded feature tuple in 3 spots, and round(.,2) on returns — all fair. Leaving them out of this PR to keep it to the consistency fix; happy to follow up separately.

[warren618]

Merged 🎉 Thanks @Robin1987China — carrying the RSI / prior-return bounds through into the generated SignalEngine is exactly the follow-through #302 was missing, and the single-cluster fallback regression test is a nice catch so sparse journals still pick up the bounds. Shipped to main in 18b6247.