Build v7 lead scoring dataset with causal trap and canonical validation

[shaypal5]

Summary

• Purely causal leakage trap: Removed boost_leakage_trap() and all label-conditioned Poisson noise from v6. The v7 trap (__leakage__touches_post_snapshot_21_90) counts raw simulated post-snapshot touches — predictive only through shared latent confounders, not label injection.
• Engine: follow-up ramp mechanism: LatentDecayIntensity gains followup_boost_after_day, followup_boost_factor, followup_ramp_days, and followup_latent_weights parameters. After day 20, sales teams increase engagement using different latent dimensions (budget_readiness, process_maturity) than pre-snapshot features use (engagement_propensity, account_fit), giving the trap genuinely marginal information.
• Documentation alignment: Snapshot day, region values, seniority levels, and all column metadata in RELEASE_v7.md exactly match the generated data (validated programmatically).
• Honest trap thresholds: Mean delta ≥ 0.008 (actual: 0.0123), min delta ≥ 0.002 (actual: 0.0048), 10/10 seeds positive. Calibrated for a purely causal trap — no artificial label boost.

Key metrics

Metric	Value
Baseline LR AUC (seed 42)	0.625
GBM improvement (5-seed avg)	+0.059
Trap delta (10-seed mean)	+0.0123
Trap delta (10-seed min)	+0.0048
Value-aware uplift K=25	+38.3%
Cohort split AUC drop	0.066
All mandatory checks	✅ Pass

Files changed

Engine (2 files):

• leadforge/mechanisms/counts.py — follow-up ramp in LatentDecayIntensity
• leadforge/mechanisms/policies.py — _FOLLOWUP_LATENT_WEIGHTS per motif family

Pipeline (1 file):

• leadforge/pipelines/build_v7.py — v7 pipeline functions (no boost_leakage_trap)

Scripts (3 files):

• scripts/build_v7_snapshot.py — CLI build script
• scripts/validate_v7_dataset.py — full validator with canonical LR pipeline
• scripts/quick_baseline_eval_v7.py — multi-model baseline evaluation

Datasets (3 files):

• lead_scoring_intro/lead_scoring_intro_v7.csv — 1000 × 20 (student)
• lead_scoring_intro/lead_scoring_intro_v7_instructor.csv — 1000 × 21 (instructor)
• lead_scoring_intro/validation_v7_report.json — full validation output

Docs (2 files):

• lead_scoring_intro/RELEASE_v7.md — complete release notes
• lead_scoring_intro/BACKGROUND_v7.md — student-facing business context

Tests (2 files):

• tests/scripts/test_build_v7_snapshot.py — 32+ pipeline tests
• tests/mechanisms/test_mechanisms.py — 9 new follow-up ramp tests

CI (1 file):

• .github/workflows/ci.yml — validate-dataset-v7 job

Test plan

• All 839 tests pass (pytest)
• ruff check . clean
• python scripts/validate_v7_dataset.py — all mandatory checks pass
• Trap is purely causal: test_no_label_injection verifies source code doesn't reference converted
• Student/instructor parity: shared columns identical, instructor has exactly 1 trap column
• CI passes on this PR

🤖 Generated with Claude Code

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[shaypal5]

Self-review from a skeptical senior-dev perspective. Posting critical remarks to keep us honest.

[shaypal5]

Whole-file duplication from v6.

This file is nearly identical to build_v6.py — the only material difference is the removal of boost_leakage_trap. Functions like derive_features, softcap_expected_acv, assign_acquisition_wave, subsample, inject_missingness, and rename_and_select are verbatim copies.

The same copy-paste pattern extends to validate_v7_dataset.py (~810 lines duplicated from v6) and quick_baseline_eval_v7.py.

When a bug is found in subsample or inject_missingness, it must be fixed in every build_vN.py independently. These shared functions belong in leadforge/pipelines/common.py with version-specific modules importing from it. The v7 module would then only contain the delta (removal of boost_leakage_trap, different constants).

[shaypal5]

9 constructor parameters — group the follow-up ramp into a dataclass.

The four followup_* parameters are a cohesive unit describing a single concept (ramp behavior). A FollowupRampConfig dataclass would:

• Make the call site in policies.py readable (one object vs. a wall of kwargs)
• Allow validation of the ramp config in isolation
• Make None semantics cleaner: followup_config: FollowupRampConfig | None = None vs. 4 independent None/default values that must be coordinated

[shaypal5]

subsample can silently return fewer than n rows.

When there aren't enough negatives (the n_neg = min(...) path), the total n_pos + n_neg may be less than the requested n. The docstring says "Stratified subsample to n rows" but this is not guaranteed. This is the same latent bug inherited from v6 by copy-paste.

The function should either enforce the total count (adjusting both sides) or raise if it can't meet the request.

[shaypal5]

Feature list duplication — 3 independent definitions.

CAT_FEATURES, NUM_FEATURES, BINARY_FEATURES are defined independently here, in quick_baseline_eval_v7.py, and as FINAL_COLUMNS_STUDENT in build_v7.py. If a column is added or renamed, all three must be updated.

The validator and eval scripts should import from leadforge.pipelines.build_v7 (or a shared constants module).

[github-actions]

pr-agent-context report:

This run includes unresolved review comments on PR #50 in repository https://github.com/leadforge-dev/leadforge

For each unresolved review comment, recommend one of: resolve as irrelevant, accept and implement
the recommended solution, open a separate issue and resolve as out-of-scope for this PR, accept and
implement a different solution, or resolve as already treated by the code.

After I reply with my decision per item, implement the accepted actions, resolve the corresponding
PR comments, and push all of these changes in a single commit.

# Other Review Comments

## REVIEW-1
Location: leadforge/pipelines/build_v7.py:1
URL: https://github.com/leadforge-dev/leadforge/pull/50#discussion_r3178395020
Root author: shaypal5

Comment:
    **Whole-file duplication from v6.**

    This file is nearly identical to `build_v6.py` — the only material difference is the removal of `boost_leakage_trap`. Functions like `derive_features`, `softcap_expected_acv`, `assign_acquisition_wave`, `subsample`, `inject_missingness`, and `rename_and_select` are verbatim copies.

    The same copy-paste pattern extends to `validate_v7_dataset.py` (~810 lines duplicated from v6) and `quick_baseline_eval_v7.py`.

    When a bug is found in `subsample` or `inject_missingness`, it must be fixed in every `build_vN.py` independently. These shared functions belong in `leadforge/pipelines/common.py` with version-specific modules importing from it. The v7 module would then only contain the delta (removal of `boost_leakage_trap`, different constants).

## REVIEW-2
Location: leadforge/mechanisms/counts.py:184
URL: https://github.com/leadforge-dev/leadforge/pull/50#discussion_r3178395022
Root author: shaypal5

Comment:
    **9 constructor parameters — group the follow-up ramp into a dataclass.**

    The four `followup_*` parameters are a cohesive unit describing a single concept (ramp behavior). A `FollowupRampConfig` dataclass would:
    - Make the call site in `policies.py` readable (one object vs. a wall of kwargs)
    - Allow validation of the ramp config in isolation
    - Make `None` semantics cleaner: `followup_config: FollowupRampConfig | None = None` vs. 4 independent `None`/default values that must be coordinated

## REVIEW-3
Location: leadforge/mechanisms/counts.py:205
URL: https://github.com/leadforge-dev/leadforge/pull/50#discussion_r3178395024
Root author: shaypal5

Comment:
    **No validation on new parameters.**

    The constructor validates `base_rate > 0`, `0 < decay_factor <= 1`, and `floor_rate >= 0`, but doesn't validate any of the new follow-up params:
    - `followup_ramp_days > 0` (0 gives `max(1, 0) = 1` — works but surprising)
    - `followup_boost_factor >= 1.0` (< 1.0 means boost *decreases* over time — may be valid but never tested)
    - `followup_boost_after_day >= 0`

    This breaks the validation pattern established by existing parameters in the same class.

    Also: with `followup_boost_factor=10.0` in `policies.py`, the effective boost at ramp end is `1.2 * 10.0 = 12.0`. Combined with latent multipliers of 2.5+, the rate modulation is `1 + 12 * 2.5 = 31x` base rate. There's no guard or documented rationale for why 10x is reasonable.

## REVIEW-4
Location: leadforge/mechanisms/policies.py:265
URL: https://github.com/leadforge-dev/leadforge/pull/50#discussion_r3178395026
Root author: shaypal5

Comment:
    **Hardcoded magic numbers with no per-motif variation.**

    `boost=1.2`, `followup_boost_after_day=20`, `followup_boost_factor=10.0`, `followup_ramp_days=10` are hardcoded inline. Every other mechanism parameter in this file varies by motif family (conversion weights, hazard params, transition weights, touch base rates, latent weights, followup latent weights). But the ramp *dynamics* are identical for all five motifs.

    This is inconsistent: a `buying_committee_friction` world presumably has different follow-up timing than a `demo_trial_mediated` world. If this is intentional (same ramp for all motifs), it deserves a comment. If it's an oversight, it needs a per-motif table like everything else.

## REVIEW-5
Location: scripts/validate_v7_dataset.py:140
URL: https://github.com/leadforge-dev/leadforge/pull/50#discussion_r3178395028
Root author: shaypal5

Comment:
    **Binary features silently dropped from validation pipeline — reported AUCs may be understated.**

    `BINARY_FEATURES` (`opportunity_created`, `demo_completed`) are defined at line 66 but `_get_feature_cols()` only returns `cat_cols` and `num_cols`. The binary features are in `FINAL_COLUMNS_STUDENT` but never make it into the sklearn pipeline's feature matrix.

    This means the "canonical" LR baseline trains *without* two likely-informative features. The reported AUC (0.625) and all trap deltas are computed on a degraded feature set. This also affects `quick_baseline_eval_v7.py` which has the same pattern.

    These should either be added to `NUM_FEATURES` (they're 0/1 ints) or returned as a third group from `_get_feature_cols`.

## REVIEW-6
Location: tests/scripts/test_build_v7_snapshot.py:354
URL: https://github.com/leadforge-dev/leadforge/pull/50#discussion_r3178395029
Root author: shaypal5

Comment:
    **No non-trivial test for `compute_post_snapshot_touches` — the core trap function.**

    `test_empty_touches` only tests the trivial case (empty list → all zeros). There is no test that:
    - Provides real touch objects and verifies correct day filtering
    - Verifies touches on exactly `snapshot_day` are excluded (boundary condition)
    - Verifies touches on exactly the horizon boundary are included
    - Verifies correct count aggregation per lead

    This is the function that computes the leakage trap — the central novelty of v7. It deserves thorough unit tests with non-trivial inputs.

## REVIEW-7
Location: tests/scripts/test_build_v7_snapshot.py
URL: https://github.com/leadforge-dev/leadforge/pull/50#discussion_r3178395031
Status: outdated
Root author: shaypal5

Comment:
    **Source-code string matching is fragile — test behavior instead.**

    ~~~python
    source = inspect.getsource(compute_post_snapshot_touches)
    assert ".converted" not in source
    ~~~

    This misses indirect label access: `df[TARGET]`, a variable holding `"converted"`, column access through a renamed alias, etc. A proper behavioral test would construct two DataFrames with identical features but different `converted` labels, run `compute_post_snapshot_touches` on both, and assert the trap values are identical.

## REVIEW-8
Location: leadforge/pipelines/build_v7.py:236
URL: https://github.com/leadforge-dev/leadforge/pull/50#discussion_r3178395035
Root author: shaypal5

Comment:
    **`subsample` can silently return fewer than `n` rows.**

    When there aren't enough negatives (the `n_neg = min(...)` path), the total `n_pos + n_neg` may be less than the requested `n`. The docstring says "Stratified subsample to n rows" but this is not guaranteed. This is the same latent bug inherited from v6 by copy-paste.

    The function should either enforce the total count (adjusting both sides) or raise if it can't meet the request.

## REVIEW-9
Location: scripts/validate_v7_dataset.py:291
URL: https://github.com/leadforge-dev/leadforge/pull/50#discussion_r3178395039
Root author: shaypal5

Comment:
    **ML pipeline defined in 3 places — no single source of truth.**

    The "canonical" pipeline appears here (`check_tree_improvement` builds its own GBM pipeline), in `_build_pipeline()` at line 107, and again in `quick_baseline_eval_v7.py` (`_build_preprocessor`). If the canonical pipeline definition changes, it must be updated in all three places.

    Also: `check_tree_improvement` has a dead code block:
    ~~~python
    if improvement < 0.02:
        # Warning, not hard failure
        pass
    ~~~
    The warning is never emitted — `pass` does nothing. Either remove the dead branch or actually append to `errors`/return a warning.

## REVIEW-10
Location: lead_scoring_intro/RELEASE_v7.md:217
URL: https://github.com/leadforge-dev/leadforge/pull/50#discussion_r3178395042
Root author: shaypal5

Comment:
    **"AUC drop" shown as `+0.066` is confusing.**

    A "drop" with a positive sign reads as an improvement. This should be `-0.066` (indicating cohort is lower), or the label should be "AUC gap" / "AUC difference (random − cohort)" with an unsigned value.

## REVIEW-11
Location: scripts/quick_baseline_eval_v7.py:121
URL: https://github.com/leadforge-dev/leadforge/pull/50#discussion_r3178395043
Root author: shaypal5

Comment:
    **Same `clf` object reused across seeds.**

    The inner loop wraps the same `clf` object in a new `Pipeline` each iteration. sklearn `Pipeline` holds the estimator by reference, so each `fit()` mutates the shared estimator.

    The AUC measurements within the loop are correct (fit completes before predict), but `random_state=42` in the classifier means the *tree randomness* is identical across all 5 splits — only the data split varies. This may understate variance.

    Consider `clone(clf)` or constructing a fresh estimator per seed.

## REVIEW-12
Location: scripts/validate_v7_dataset.py:66
URL: https://github.com/leadforge-dev/leadforge/pull/50#discussion_r3178395044
Root author: shaypal5

Comment:
    **Feature list duplication — 3 independent definitions.**

    `CAT_FEATURES`, `NUM_FEATURES`, `BINARY_FEATURES` are defined independently here, in `quick_baseline_eval_v7.py`, and as `FINAL_COLUMNS_STUDENT` in `build_v7.py`. If a column is added or renamed, all three must be updated.

    The validator and eval scripts should import from `leadforge.pipelines.build_v7` (or a shared constants module).

Run metadata:

Tool ref: v4
Tool version: 4.0.21
Trigger: commit pushed
Workflow run: 25284529983 attempt 1
Comment timestamp: 2026-05-03T16:28:11.009991+00:00
PR head commit: 64fc53db4056d09053bf2577e154b3923b606c1c