Skip to content

Commit 841c6a8

Browse files
shaypal5claude
andauthored
fix: redact current_stage from student_public bundles (#56)
* fix: redact current_stage from student_public bundles Add an `is_leakage_trap` flag to FeatureSpec and an exposure-layer column-redaction pass that drops `leakage_risk and not is_leakage_trap` columns from student_public snapshots, task splits, feature dictionary, and dataset card. `current_stage` (true label leak) is now removed in student_public; `total_touches_all` (intentional pedagogical trap) is preserved via the new flag. - `schema/features.py`: add `is_leakage_trap` field; mark `total_touches_all`; export `STUDENT_PUBLIC_REDACTED_COLUMNS`. - `schema/dictionaries.py`: emit `is_leakage_trap` column in `feature_dictionary.csv`. - `exposure/filters.py`: `BundleFilter.redacted_columns`; populated for student_public. - `api/bundle.py`: drop redacted columns from snapshot before splits; thread the visible feature tuple to dataset card and dictionary writers. - `narrative/dataset_card.py`: accept a `features` arg so categories / leakage section reflect what is actually published. - `validation/bundle_checks.py`: new `_check_exposure_redaction()` enforces CLAUDE.md invariant #1 — redacted columns must not appear in the published task splits or dictionary. - `validation/invariants.py`: relax exposure-monotonicity from byte identity to subset-with-shared-content for `feature_dictionary.csv` and task splits, so redaction doesn't trip the check. Tests: extend `tests/schema/test_features.py` with five new assertions covering the trap flag, the redaction set, and `current_stage`'s role. All 880 existing tests still pass. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * docs: update release docs and add end-to-end redaction tests - `tests/exposure/test_redaction.py` (19 tests): bundle-level proof that `current_stage` is absent from every student_public task split, that `total_touches_all` (the pedagogical trap) is preserved, that the research_instructor bundle still contains both, that the student feature dictionary differs from the instructor's by exactly the redacted set, that shared columns hold identical values across modes, and that `validate_bundle()` flags a tampered student_public bundle whose task split still carries `current_stage`. - `scripts/build_public_release.py`: drop `_FLAT_CSV_DROP_COLS` — the bundle writer now redacts at source, so the flat CSV inherits it. - `release/README.md`, `release/HF_DATASET_CARD.md`: update column counts (33 features + 1 trap + 1 target in student_public; 34 + 1 + 1 in the instructor companion) and rewrite the leakage section to distinguish stripped columns from the deliberate trap. - `.agent-plan.md`: flip "Known issue: current_stage leakage" to resolved with a link to the redaction mechanism. Bundles regenerated; all four pass `leadforge validate`; `scripts/verify_hash_determinism.py` confirms 73/73 files identical across two consecutive builds. Full test suite: 899 passing. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * refactor: collapse two-flag redaction into prescriptive redact_in_modes Addresses senior-dev review of the previous two commits: 1. **Single source of truth.** Replaced the awkward `leakage_risk + is_leakage_trap` flag pair with a single prescriptive field `FeatureSpec.redact_in_modes: frozenset[ExposureMode]`. `leakage_risk` reverts to purely descriptive ("post-snapshot correlated"). `current_stage` carries `redact_in_modes={student_public}`; `total_touches_all` keeps `leakage_risk=True` with empty `redact_in_modes` (deliberate trap). New `redacted_columns_for(mode, features=...)` parameterizable function replaces the frozen module-level set. 2. **Validation independent of the writer.** `_check_exposure_redaction` now derives the expected redaction set directly from `LEAD_SNAPSHOT_FEATURES` via `redacted_columns_for`, *not* from `BundleFilter`. A bug in the filter no longer agrees-with-itself past the validator. Added a real regression test that writes a real student_public bundle, mutates a parquet to reinsert `current_stage`, and asserts validation fails. 3. **Self-describing manifest.** `manifest.json` now records `redacted_columns: [...]`. The validator cross-checks the manifest's declared set against the feature-spec-derived expected set; a second new test mutates the manifest to claim nothing was redacted and asserts validation flags the disagreement. 4. **Stricter exposure monotonicity.** `check_exposure_monotonicity` now asserts that `instructor_columns - student_columns` *equals* `redacted_columns_for(student_public)` — not just "is a superset". A future column drop in student_public outside the redaction set is now caught. 5. **Reverted feature_dictionary.csv schema.** The previous commit added an `is_leakage_trap` column to the published CSV without flagging the schema change. Removed: the redaction policy is package-internal and the bundle's actual published schema is observable from the parquet files and `manifest.redacted_columns`. CSV stays back-compat with prior releases (6 columns: name, dtype, description, category, is_target, leakage_risk). 6. **De-duplicated documentation.** `release/README.md` no longer maintains a per-category feature count table by hand — that data is in the auto-generated `dataset_card.md`. Removed the duplicated table; added explicit caveats covering known structural-leakage issues (event aggregates over the label window, `is_mql` zero variance, `is_sql=False` near-deterministic for non-conversion). 7. **Documented the structural follow-up.** `.agent-plan.md` now includes a clearly-scoped "Follow-up: structural leakage" section covering: (a) windowed-snapshot fix for event aggregates; (b) measured `is_sql` leakage P(conv|is_sql=False) = 0.038/0.015/0.006 across tiers; (c) `is_mql` is constant True (zero variance). Suggests filing a tracked issue (deferred — needs user confirmation per shared-state-action rules). Removed: `BundleFilter.redacted_columns` (replaced by direct calls to `redacted_columns_for(mode)` from the bundle writer). Tests: schema tests rewritten for `redact_in_modes` semantics; redaction tests updated for the new function-based API; +2 manifest-redaction tests; +1 real regression test (mutated parquet) replacing the previous manifest-string-edit test. 902 tests passing (was 899). Hash determinism verified: 73/73 files identical across two consecutive builds. All four release bundles regenerated; `feature_dictionary.csv` is now back to its pre-PR 6-column schema. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
1 parent f61e98a commit 841c6a8

14 files changed

Lines changed: 580 additions & 60 deletions

File tree

.agent-plan.md

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ First public dataset release: `leadforge-b2b-lead-scoring`. Three difficulty tie
4545
- [x] Update release/README.md — remove stale "Known limitations", add conversion rates to dataset summary
4646
- [x] Update release/HF_DATASET_CARD.md — add conversion rates to summary table
4747
- [x] Verify SHA-256 hash determinism (re-run build, compare hashes) — `scripts/verify_hash_determinism.py`; 73/73 files identical across two `build_public_release.py` runs (modulo `manifest.json`'s wall-clock `generation_timestamp`)
48+
- [x] Fix `current_stage` leakage in student_public bundles via exposure-layer redaction — `is_leakage_trap` flag distinguishes the pedagogical trap (`total_touches_all`) from true label leaks; `BundleFilter.redacted_columns` strips the latter; `validate_bundle()` enforces the invariant. 73/73 hash-determinism preserved.
4849
- [ ] Upload to Kaggle and HuggingFace
4950
- [ ] Announce
5051

@@ -61,9 +62,19 @@ First public dataset release: `leadforge-b2b-lead-scoring`. Three difficulty tie
6162
- [x] Calibration across 20 seeds × 5 motif families: intro mean 43%, intermediate mean 22%, advanced mean 9%
6263
- [x] All 865 tests pass
6364

64-
### Known issue: `current_stage` leakage at 90-day horizon
65+
### Resolved (partial): `current_stage` leakage at 90-day horizon
6566

66-
The full bundle snapshot includes `current_stage` which at day 90 contains terminal stages (`closed_won`/`closed_lost`). This perfectly encodes the label. The flat CSV export drops it; the Parquet task splits retain it with documentation. A proper fix (windowed snapshot or column redaction in the exposure layer) is deferred.
67+
Deterministic leak fixed via exposure-layer redaction. `FeatureSpec` now carries an explicit `redact_in_modes: frozenset[ExposureMode]` field — *prescriptive* — alongside the descriptive `leakage_risk` flag. `current_stage` is marked `redact_in_modes={ExposureMode.student_public}`; the writer queries `redacted_columns_for(mode)` and strips matching columns from the snapshot, task splits, and feature dictionary before they hit disk. The pedagogical trap `total_touches_all` is preserved in all modes (no entry in `redact_in_modes`). The manifest records `redacted_columns: [...]` so the bundle is self-describing. `validate_bundle()` cross-checks parquet schemas, feature dictionary, and the manifest's declared redaction set against `redacted_columns_for(mode)` derived independently from the feature spec. Hash-determinism preserved (73/73 identical across builds).
68+
69+
### Follow-up: structural leakage in `student_public` bundles (open)
70+
71+
Stripping `current_stage` addresses the deterministic label-encoding leak but does **not** make the released bundle structurally leakage-free. Three concerns to address in a follow-up PR:
72+
73+
1. **Event-aggregate features are computed over the label window.** `touch_count`, `session_count`, `pricing_page_views`, `expected_acv`, `days_since_last_touch`, etc. all aggregate events in `[lead_created_at, lead_created_at + 90d]`, the same window over which the label resolves. They correlate with post-conversion activity. The structural fix is a windowed snapshot (`snapshot_day=N` with `N < label_window_days`), as v6/v7 datasets already do at day 14/20. This shifts every feature value and every conversion rate in the release bundles, so it's deferred to its own PR with a coordinated documentation update.
74+
2. **`is_sql=False` is near-deterministic for non-conversion.** Measured on the regenerated bundle: P(converted | is_sql=False) = 0.038 (intro), 0.015 (intermediate), 0.006 (advanced). At advanced tier it effectively encodes the negative class. Either redact `is_sql` in `student_public` (probably correct) or accept it as a strong feature with documentation. Decide alongside #1.
75+
3. **`is_mql` is a constant `True`.** Zero variance feature in all three tiers. Should be removed from the snapshot or, if it can ever be False under some recipe, the simulator should produce that variance.
76+
77+
Suggested action: open one tracked GitHub issue covering all three (currently no issue exists; user has standing instruction not to file without confirmation).
6778

6879
---
6980

leadforge/api/bundle.py

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
from leadforge.render.snapshots import build_snapshot
2424
from leadforge.render.tasks import write_task_splits
2525
from leadforge.schema.dictionaries import write_feature_dictionary
26+
from leadforge.schema.features import LEAD_SNAPSHOT_FEATURES, redacted_columns_for
2627
from leadforge.schema.tables import write_parquet
2728
from leadforge.schema.tasks import task_manifest_for_config
2829

@@ -72,6 +73,12 @@ def write_bundle(
7273

7374
# ------------------------------------------------------------------
7475
# 2. Snapshot + task splits → tasks/
76+
#
77+
# Apply exposure-mode redaction here (rather than in apply_exposure)
78+
# so that the manifest's per-file SHA-256 hashes reflect the published
79+
# column set without a post-write rewrite step. The redacted column
80+
# set is derived from the canonical feature spec — the same source
81+
# of truth the validator uses to check bundles.
7582
# ------------------------------------------------------------------
7683
snapshot = build_snapshot(
7784
result,
@@ -80,16 +87,28 @@ def write_bundle(
8087
difficulty_params=config.difficulty_params,
8188
seed=config.seed,
8289
)
90+
redacted = redacted_columns_for(config.exposure_mode)
91+
if redacted:
92+
drop_cols = [c for c in redacted if c in snapshot.columns]
93+
if drop_cols:
94+
snapshot = snapshot.drop(columns=drop_cols)
95+
visible_features = tuple(f for f in LEAD_SNAPSHOT_FEATURES if f.name not in redacted)
96+
8397
task = task_manifest_for_config(config.primary_task, config.label_window_days)
8498
task_row_counts = write_task_splits(snapshot, root / "tasks", seed=config.seed, task=task)
8599

86100
# ------------------------------------------------------------------
87101
# 3. Dataset card and feature dictionary
88102
# ------------------------------------------------------------------
89103
(root / "dataset_card.md").write_text(
90-
render_dataset_card(bundle.spec, task_manifest=task, table_counts=table_row_counts)
104+
render_dataset_card(
105+
bundle.spec,
106+
task_manifest=task,
107+
table_counts=table_row_counts,
108+
features=visible_features,
109+
)
91110
)
92-
write_feature_dictionary(root / "feature_dictionary.csv")
111+
write_feature_dictionary(root / "feature_dictionary.csv", features=visible_features)
93112

94113
# ------------------------------------------------------------------
95114
# 4. Exposure metadata (research_instructor only)
@@ -106,5 +125,6 @@ def write_bundle(
106125
task_row_counts={task.task_id: task_row_counts},
107126
bundle_root=root,
108127
generation_timestamp=generation_timestamp,
128+
redacted_columns=sorted(redacted),
109129
)
110130
write_manifest(manifest, root)

leadforge/exposure/filters.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,12 @@
44
:class:`BundleFilter` that governs which artefacts are written when
55
:func:`~leadforge.api.bundle.write_bundle` produces an output bundle.
66
7+
The per-feature redaction policy lives separately on
8+
:attr:`leadforge.schema.features.FeatureSpec.redact_in_modes` and is queried
9+
via :func:`leadforge.schema.features.redacted_columns_for`. ``BundleFilter``
10+
deliberately does *not* duplicate that information so that the writer and
11+
the validator both consult the same source of truth.
12+
713
Adding a new mode: define its ``BundleFilter`` entry in ``FILTERS``.
814
"""
915

@@ -16,7 +22,7 @@
1622

1723
@dataclass(frozen=True)
1824
class BundleFilter:
19-
"""Rules that govern bundle publication for one :class:`ExposureMode`.
25+
"""Mode-level publication policy.
2026
2127
Attributes:
2228
write_metadata: Whether to create ``metadata/`` with hidden-truth

leadforge/narrative/dataset_card.py

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
from collections import Counter
1010
from typing import TYPE_CHECKING
1111

12-
from leadforge.schema.features import LEAD_SNAPSHOT_FEATURES
12+
from leadforge.schema.features import LEAD_SNAPSHOT_FEATURES, FeatureSpec
1313

1414
if TYPE_CHECKING:
1515
from leadforge.core.models import WorldSpec
@@ -20,6 +20,7 @@ def render_dataset_card(
2020
world_spec: WorldSpec,
2121
task_manifest: TaskManifest | None = None,
2222
table_counts: dict[str, int] | None = None,
23+
features: tuple[FeatureSpec, ...] = LEAD_SNAPSHOT_FEATURES,
2324
) -> str:
2425
"""Return a Markdown dataset card string for *world_spec*.
2526
@@ -31,6 +32,10 @@ def render_dataset_card(
3132
table_counts: Optional mapping of table name → row count. When
3233
provided, the table inventory section renders actual counts
3334
instead of a placeholder.
35+
features: Feature spec tuple to render in the categories / leakage
36+
sections. Defaults to the canonical list; pass the redacted
37+
tuple when rendering an exposure-filtered bundle so the card
38+
describes only what is actually present.
3439
3540
Sections:
3641
- Header (recipe id, version, seed, exposure mode)
@@ -149,18 +154,16 @@ def render_dataset_card(
149154
# ------------------------------------------------------------------
150155
lines += ["## Feature categories", ""]
151156
category_counts: Counter[str] = Counter()
152-
for feat in LEAD_SNAPSHOT_FEATURES:
157+
for feat in features:
153158
category_counts[feat.category] += 1
154159
lines += [
155160
"| Category | Count | Examples |",
156161
"|---|---:|---|",
157162
]
158163
for cat, count in category_counts.items():
159-
examples = [
160-
f.name for f in LEAD_SNAPSHOT_FEATURES if f.category == cat and not f.is_target
161-
][:3]
164+
examples = [f.name for f in features if f.category == cat and not f.is_target][:3]
162165
lines.append(f"| {cat} | {count} | {', '.join(examples)} |")
163-
leakage_cols = [f.name for f in LEAD_SNAPSHOT_FEATURES if f.leakage_risk]
166+
leakage_cols = [f.name for f in features if f.leakage_risk]
164167
if leakage_cols:
165168
lines += [
166169
"",

leadforge/render/manifests.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ def build_manifest(
3535
task_row_counts: dict[str, dict[str, int]],
3636
bundle_root: Path,
3737
generation_timestamp: str | None = None,
38+
redacted_columns: list[str] | None = None,
3839
) -> dict[str, Any]:
3940
"""Build the bundle manifest dict.
4041
@@ -49,13 +50,20 @@ def build_manifest(
4950
task_row_counts: Mapping of task_id → {split_name → row count}.
5051
bundle_root: Root directory of the written bundle.
5152
generation_timestamp: ISO-8601 UTC timestamp string. Defaults to now.
53+
redacted_columns: Sorted list of column names that the bundle writer
54+
removed from snapshot / task splits / feature dictionary for
55+
this exposure mode. Recorded in the manifest so consumers
56+
(and the validator) can audit redaction without inspecting
57+
package internals. Defaults to ``[]`` (nothing redacted).
5258
5359
Returns:
5460
A JSON-serialisable dict ready to be written as ``manifest.json``.
5561
"""
5662
if generation_timestamp is None:
5763
generation_timestamp = datetime.now(UTC).isoformat(timespec="seconds")
5864

65+
redacted_columns_list = sorted(redacted_columns) if redacted_columns else []
66+
5967
# Build table entries with row counts and file hashes.
6068
tables: dict[str, Any] = {}
6169
for table_name, row_count in table_row_counts.items():
@@ -91,6 +99,7 @@ def build_manifest(
9199
"primary_task": config.primary_task,
92100
"label_window_days": config.label_window_days,
93101
"motif_family": world_graph.motif_family,
102+
"redacted_columns": redacted_columns_list,
94103
"tables": tables,
95104
"tasks": tasks,
96105
}

leadforge/schema/dictionaries.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,13 @@ def feature_dictionary_df(
2323
2424
Columns: name, dtype, description, category, is_target, leakage_risk.
2525
26+
The redaction policy (``FeatureSpec.redact_in_modes``) is intentionally
27+
*not* serialised here: it is package-internal state, and which columns
28+
a given bundle actually published is observable from the bundle's
29+
schema and from ``manifest.redacted_columns``. Keeping this CSV's
30+
column set stable preserves backward compatibility with downstream
31+
consumers that parse it strictly.
32+
2633
Args:
2734
features: Ordered tuple of :class:`~leadforge.schema.features.FeatureSpec`
2835
objects. Defaults to the canonical lead snapshot feature list.

leadforge/schema/features.py

Lines changed: 46 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,31 @@
88

99
from __future__ import annotations
1010

11-
from dataclasses import dataclass
11+
from dataclasses import dataclass, field
12+
13+
from leadforge.core.enums import ExposureMode
1214

1315

1416
@dataclass(frozen=True)
1517
class FeatureSpec:
1618
"""Metadata for one column in the lead snapshot table.
1719
20+
Two concerns are kept deliberately separate:
21+
22+
- :attr:`leakage_risk` is *descriptive*: the value of this column is
23+
computed from events that may post-date the snapshot anchor and so
24+
correlates with the label. It is informational metadata for
25+
downstream consumers and is preserved in the published feature
26+
dictionary.
27+
- :attr:`redact_in_modes` is *prescriptive*: the bundle writer must
28+
strip this column from any export whose mode is in this set.
29+
30+
These can disagree: ``total_touches_all`` is ``leakage_risk=True``
31+
(it does encode post-snapshot information) but
32+
``redact_in_modes=frozenset()`` (it is deliberately retained as a
33+
pedagogical trap). Conversely a recipe could redact a column that
34+
is not itself leakage-risky for unrelated policy reasons.
35+
1836
Attributes:
1937
name: Column name as it appears in the Parquet file.
2038
dtype: Pandas-compatible dtype string (``"string"``, ``"Int64"``,
@@ -23,8 +41,10 @@ class FeatureSpec:
2341
category: Logical grouping (``"account"``, ``"contact"``,
2442
``"lead_meta"``, ``"engagement"``, ``"sales"``, ``"target"``).
2543
is_target: True for the label column only.
26-
leakage_risk: True if the column could contain post-snapshot-anchor
27-
information and must be excluded from student_public exports.
44+
leakage_risk: Descriptive — this column is post-snapshot correlated.
45+
redact_in_modes: Prescriptive — exposure modes in which the
46+
bundle writer must strip this column from snapshot, task
47+
splits, and feature dictionary.
2848
"""
2949

3050
name: str
@@ -33,6 +53,7 @@ class FeatureSpec:
3353
category: str
3454
is_target: bool = False
3555
leakage_risk: bool = False
56+
redact_in_modes: frozenset[ExposureMode] = field(default_factory=frozenset)
3657

3758

3859
# ---------------------------------------------------------------------------
@@ -122,6 +143,7 @@ class FeatureSpec:
122143
"a windowed snapshot.",
123144
"lead_meta",
124145
leakage_risk=True,
146+
redact_in_modes=frozenset({ExposureMode.student_public}),
125147
),
126148
FeatureSpec(
127149
"is_mql",
@@ -235,7 +257,7 @@ class FeatureSpec:
235257
"revenue band midpoint heuristic (NaN if neither available).",
236258
"sales",
237259
),
238-
# -- Leakage trap --
260+
# -- Pedagogical leakage trap (deliberately retained in all modes) --
239261
FeatureSpec(
240262
"total_touches_all",
241263
"Int64",
@@ -254,3 +276,23 @@ class FeatureSpec:
254276
is_target=True,
255277
),
256278
)
279+
280+
281+
def redacted_columns_for(
282+
mode: ExposureMode,
283+
features: tuple[FeatureSpec, ...] = LEAD_SNAPSHOT_FEATURES,
284+
) -> frozenset[str]:
285+
"""Return the set of column names that must be stripped from *mode* exports.
286+
287+
The redaction policy is encoded per-feature in
288+
:attr:`FeatureSpec.redact_in_modes`. Callers (the bundle writer, the
289+
validation check) all derive their answer from this single function, so
290+
a single source of truth governs both producing and verifying bundles.
291+
292+
Args:
293+
mode: The exposure mode being published.
294+
features: Feature spec tuple to consult. Defaults to the canonical
295+
lead snapshot list; callable with a custom tuple for tests or
296+
future per-recipe feature sets.
297+
"""
298+
return frozenset(f.name for f in features if mode in f.redact_in_modes)

0 commit comments

Comments
 (0)