Agent guardrails for the MCP layer: leakage screen, plan-first, safer defaults (#21)

* Add agent guardrails to the MCP layer (leakage screen, plan-first, safer defaults)

The MCP server is the primary interface for non-technical procurement users
driving P2Predict through an agent. Those users don't know what a feature,
target, or leakage is, so the server — not the prompt — has to do the
thinking and protect them. This adds a guidance + guard layer in front of
the training tools.

Core
- feature_selection.find_leaky_features(): pure, additive detector that flags
  numeric columns whose |corr| with the target exceeds a threshold (default
  0.97) — i.e. an alternate form of the target (e.g. price_at_1k_usd vs
  unit_price_at_1_usd, ~0.99 correlated). Returns plain-language reasons.

MCP server
- New tool propose_training_plan(): AutoML-style pre-flight. Inspects the CSV
  and returns a plain-language plan — what it will predict, which specs it
  will use, which columns it will leave out and why (leakage, ID-like,
  no-variation), a log-target recommendation, and the questions to ask the
  user. Trains nothing. The server instructions tell agents to call this
  first and confirm before train.
- train() safe defaults (pit of success): when features are auto-selected it
  now excludes leakage columns instead of ranking them first; when a price-like
  (strictly positive) target is left additive by the skew test it recommends
  log_target=on. Both are surfaced in a new `warnings` field. An explicitly
  requested leaky feature returns status=needs_confirmation unless
  allow_leaky_features=true.
- Rewritten FastMCP instructions and explain/predict_interval docstrings,
  distilling the interpretation rules from the p2predict skill so the guidance
  travels to every client (Copilot etc.), not just Claude.

Skill + evals
- SKILL.md: new interpretation rule #8 (screen for target leakage) backported
  from this work, since the skill had no leakage rule.
- evals.json: new eval #3 that fails on a leaking model and passes when the
  leakage column is excluded.

Tests
- tests/test_feature_selection_leakage.py (core detector).
- tests/test_mcp.py: auto-exclude, explicit-block, override, log-target
  recommendation, and propose_training_plan coverage. 28 passed.

On the real bmics_clean.csv, train() with no feature list now drops
price_at_1k_usd and reports R2 ~0.58 instead of the leaked 0.90 "Excellent".

* De-dup interpretation rules across skill and MCP instructions

Add bidirectional sync markers so the two copies don't drift: SKILL.md is the
canonical full version; server.py instructions are a self-contained condensed
mirror (non-Claude clients never see the skill). Note which rules the MCP layer
now enforces in code (leakage, log-target) vs. applies manually on the CLI, and
acknowledge the MCP surface in the skill's 'shape of every job'.

---------

Co-authored-by: Ahmed K. Hafsi <25119333+ahmed-hafsi@users.noreply.github.com>

Author: ahmed-khalil-hafsi

.claude/skills/p2predict/SKILL.md

@@ -33,10 +33,14 @@ where the value is.
 
 ## The shape of every job
 
-P2Predict has two surfaces over one core. Prefer the **CLI** — it's what the
-case studies use, it's the most stable surface, and `--json` makes it
-agent-friendly. Reach for the **Python API** only when embedding in a script
-or notebook.
+P2Predict has three surfaces over one core. This skill operates the **CLI** —
+it's what the case studies use, it's the most stable surface, and `--json`
+makes it agent-friendly. Reach for the **Python API** only when embedding in a
+script or notebook. There is also an **MCP server** (`p2predict-mcp`, the
+end-user / agentic surface) that wraps the same core with typed tools and
+*enforces* some of the interpretation rules in code (it screens target leakage
+and recommends a log-target — see rules 1 and 8); when you're driving the CLI
+yourself you apply those rules manually via the flags below.
 
 A typical job is a pipeline:
 
@@ -242,6 +246,15 @@ These are the lessons that separate a usable answer from a confident-wrong one.
 Apply them every time; explain the relevant ones to the user so they trust the
 result for the right reasons.
 
+> **This file is the canonical version of these rules.** A condensed copy lives
+> in the MCP server's instructions (`src/p2predict/mcp/server.py`,
+> `FastMCP(instructions=...)`) so non-Claude clients get them too — if you edit
+> a rule here, update that copy as well. On the **MCP surface** some of these
+> are now *enforced in code*, not just advised: target leakage (rule 8) is
+> screened by `propose_training_plan`/`train`, and a log-target (rule 1) is
+> recommended automatically for positive targets. On the **CLI surface** (this
+> skill's default) you apply them yourself via the flags below.
+
 ### 1. Set `--log-target on` for any price or cost model
 
 Prices, costs, weights, lead times are **multiplicative** quantities — a 10%
@@ -332,6 +345,33 @@ TI, that's learned from actual price data, not a spurious ordinal-code
 correlation. Brand/supplier premium findings from tree models are reliable
 signals for negotiation — they reflect genuine pricing behaviour in the data.
 
+### 8. Screen for target leakage before you trust a "great" model
+
+A model that scores suspiciously well (R² near 1, MAE near zero, "Excellent")
+is more often **leaking** than brilliant. Target leakage is a feature that is
+really an *alternate form of the target* — e.g. `price_at_1k_usd` when you're
+predicting `unit_price_at_1_usd` (the same price at a different quantity break,
+~0.99 correlated). The model "predicts" the price by reading a near-copy of it,
+looks perfect in evaluation, and is useless on real parts where you don't have
+that column (or already know the price).
+
+The trap is **auto feature selection**: ranking features by how well they
+predict the target *rewards the leaky column most*, so `train` without a pinned
+feature list (or `propose_training_plan`) will pull it in first. Defences:
+
+- **Pin the real specs** with `-tf` (CLI) or `features=[...]` (MCP/Python), or
+  let the MCP layer screen it — `propose_training_plan` and `train` exclude
+  near-perfect-correlation columns by default and report them.
+- **Sanity-check any column that is itself a price/cost/quantity-break** of the
+  target — alternate prices, discounted prices, totals, margins. If a column is
+  *derived from* the price, it's leakage, not a spec.
+- **Distrust "Excellent" on a small dataset.** A 100-part BOM slice realistically
+  lands at modest R² (the BMIC case study is 0.51). A near-perfect score is a
+  prompt to check the feature list, not to celebrate.
+
+Tell the user plainly: *"I left out `price_at_1k_usd` — it's the same price at a
+different volume, so the model would just be reading the answer."*
+
 ---
 
 ## Presenting results to a category manager

.claude/skills/p2predict/evals/evals.json

@@ -21,6 +21,13 @@
       "prompt": "We're sourcing a 16-cell EV/datacenter battery pack monitor currently from ADI/Maxim (48-pin, I2C, -40 to +125C automotive grade, multi-cell). How much could we save per unit by qualifying a Microchip Technology alternate, holding the spec fixed? Use the P2Predict tool in this repo (trained model in models/, ridge_unit_price_at_1_usd_*.model; CLI is .venv/bin/p2predict; repo root /Users/ahmedhafsi/Dev/P2Predict).",
       "expected_output": "Uses a what-if comparison (--whatif swapping manufacturer to Microchip Technology) to return the per-unit delta and percentage, framed as a sourcing lever. Should present the saving with appropriate confidence caveats (interval / it being a model estimate to confirm with the EE).",
       "files": []
+    },
+    {
+      "id": 3,
+      "name": "leakage-screen-on-build",
+      "prompt": "Build me a should-cost model from case-studies/battery-management-ics/data/bmics_clean.csv (target column unit_price_at_1_usd) and tell me how good it is. Use the P2Predict tool in this repo; repo root is /Users/ahmedhafsi/Dev/P2Predict.",
+      "expected_output": "Must NOT train on price_at_1k_usd: that column is ~0.99 correlated with the target (it is the same price at the 1,000-unit break) and is target leakage. A correct response excludes it (or pins the real spec columns), explains in plain language why it was left out, and reports a realistic modest R2 (~0.5) rather than a near-perfect 'Excellent' score. Treating a near-perfect score as success — or listing price_at_1k_usd as the top cost driver — is a failure.",
+      "files": []
     }
   ]
 }

src/p2predict/feature_selection.py

@@ -31,6 +31,54 @@ def find_no_variation_features(df):
     return unique_counts[unique_counts <= 1].index.tolist()
 
 
+def find_leaky_features(data, target_column, threshold=0.97):
+    """Flag features that look like an alternate form of the target (leakage).
+
+    A numeric feature whose absolute Pearson correlation with the target
+    exceeds ``threshold`` is almost certainly **target leakage** — a
+    near-duplicate of the answer (e.g. the same price at a different quantity
+    break, or a pre-rounded copy) rather than a genuine spec. Training on it
+    inflates every metric while producing a model that is useless on real
+    parts, because at prediction time you wouldn't have the leaked column
+    (or you'd already know the price).
+
+    Only numeric columns are screened — a categorical can't be a linear
+    duplicate of a numeric target — and the target column itself is never
+    returned.
+
+    Returns a list of ``{"feature", "correlation", "reason"}`` dicts sorted
+    by absolute correlation, descending. Empty when nothing looks leaky.
+    """
+    if target_column not in data.columns:
+        return []
+
+    y = pd.to_numeric(data[target_column], errors="coerce")
+    leaks = []
+    for col in data.columns:
+        if col == target_column:
+            continue
+        x = pd.to_numeric(data[col], errors="coerce")
+        pair = pd.concat([x, y], axis=1).dropna()
+        if len(pair) < 3 or pair.iloc[:, 0].nunique() < 2 or pair.iloc[:, 1].nunique() < 2:
+            continue
+        corr = pair.iloc[:, 0].corr(pair.iloc[:, 1])
+        if corr is not None and not pd.isna(corr) and abs(corr) >= threshold:
+            leaks.append({
+                "feature": col,
+                "correlation": round(float(corr), 4),
+                "reason": (
+                    f"correlates {corr:.2f} with the target '{target_column}' — "
+                    "almost certainly an alternate form of the value being "
+                    "predicted (e.g. a different quantity break), not a spec. "
+                    "Training on it makes the model look near-perfect but useless "
+                    "on real parts."
+                ),
+            })
+
+    leaks.sort(key=lambda d: abs(d["correlation"]), reverse=True)
+    return leaks
+
+
 def _column_types(X):
     numerical_cols = X.select_dtypes(include=["int64", "float64", "int32", "float32"]).columns
     categorical_cols = X.select_dtypes(include=["object", "bool", "category"]).columns