feat(features): pydantic configs + PRP set for phase 2 feature wiring (#109)#110
Conversation
…sets (#109) Schema-only, additive PR landing the foundation for PRP-3.1B/C/D parallel implementation: - LifecycleConfig: include_days_since_launch, include_days_since_discontinue, lag_days (1-30). Continuous-only encoding per PRP-3.1 decisions log §1. - ReplenishmentConfig: include_days_since_last, include_count_window, lag_days, count_window_days (7-60). - PromotionConfig: kinds_to_track (tuple[Literal["pct_off","bogo","bundle", "markdown"], ...]) with non-empty/unique validator, include_active, include_intensity, lag_days. Generalized from MarkdownConfig per decisions §3. FeatureSetConfig gains three optional sub-config slots (all `T | None = None`, positioned between exogenous_config and imputation_config) and get_enabled_features() emits "lifecycle","replenishment","promotion" tokens. Phase 2 DataFrame fixtures (phase2_product_attrs_df, phase2_replenishment_events_df, phase2_promotion_rows_df) added to conftest.py with sequential / derivable values so downstream leakage tests can mathematically detect contamination. Behavioral note: FeatureConfigBase.config_hash() now uses model_dump_json(exclude_none=True) so that adding new optional fields is hash-invariant for callers that don't set them (additive-contract guarantee required by PRD §6/§11; previously violated by Pydantic's default null-key serialization). New baseline hash for FeatureSetConfig(name="x") is 6c12b1a783eccdd4 and pinned via a snapshot test. Tests: 46 passing in test_schemas.py (incl. 18 new cases across the three new Configs + 3 new TestFeatureSetConfig cases). Full featuresets module sweep: 75 passing, zero regressions. Forecasting + registry consumers of config_hash: 188 passing. No service.py edits, no routes.py edits, no DB migration.
Lands the 5-slice PRP set defining the parallel execution plan for issue #109 (wire phase 2 seeder columns into time-safe featuresets): - PRP-3.1A (3.1A): pydantic configs + phase 2 fixtures (schema-only, IMPLEMENTED by the preceding commit). - PRP-3.1B: lifecycle compute method (days_since_launch / days_since_discontinue, continuous-only encoding). - PRP-3.1C: replenishment compute method (days_since_last_replenishment + rolling event count via async event-table JOIN). - PRP-3.1D: promotion compute method (per-kind active + intensity features, chain-wide vs store-specific JOIN, NULL-discount handling). - PRP-3.1E: end-to-end integration test + docs update (PHASE/3-FEATURE_ENGINEERING.md, _base/DOMAIN_MODEL.md, examples). Recommended sequence: A → B → (C ∥ D) → E. Each downstream PRP cites exact file paths + line numbers in the current repo, has executable validation gates (Level 1-5: ruff / mypy+pyright / unit / leakage / HTTP smoke), and targets a 9/10 confidence score. Each PRP is self-contained: an implementing agent can read one PRP + edit 2-3 files + run 5-6 commands and ship a green PR.
There was a problem hiding this comment.
Sorry @w7-mgfcode, your pull request is larger than the review limit of 150000 diff characters
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary
Lands the foundation of the PRP-3.1 umbrella (issue #109 — wire Phase 2 seeder columns into time-safe featuresets):
LifecycleConfig,ReplenishmentConfig,PromotionConfig), 3 optionalFeatureSetConfigsub-config slots, extendedget_enabled_features(), 3 Phase 2 DataFrame fixtures, 18 new test cases. No service/routes edits. No DB migration.This PR alone is a schema-only additive diff (+370 / -1 in
app/features/featuresets/). It unblocks PRP-3.1B/C/D to run as parallel branches offdevwithout conflicting onschemas.py.What changed
app/features/featuresets/schemas.pyapp/features/featuresets/tests/conftest.pyapp/features/featuresets/tests/test_schemas.pyPRPs/PRP-3.1A-…md+PRP-3.1B-…md+PRP-3.1C-…md+PRP-3.1D-…md+PRP-3.1E-…mdNotable decisions made during execution
config_hash()usesexclude_none=True. The PRP's "byte-identical pre/post PR" additive-contract invariant was unsatisfiable without this fix: Pydantic's defaultmodel_dump_json()serializesNone-defaulted fields asnullkeys, so adding optional fields changes the dump regardless of whether the caller sets them. The 1-line fix inFeatureConfigBase.config_hash()makes the additive contract TRUE going forward. New baseline hash forFeatureSetConfig(name="x")is6c12b1a783eccdd4and pinned via a snapshot test (test_config_hash_unchanged_when_phase2_omitted). Verified safe: no existing test pins a literal hash value; downstream consumers (forecasting/registry) all green.test_schemas.py(+168 vs ~50 target): 18 test cases across the 3 new Configs instead of the PRP's "≥4 cases" minimum. Schema is the contract for PRP-3.1B/C/D; more validation cases now means downstream slices land cleaner.tuple[str, ...]validator signature forPromotionConfig.kinds_to_track. Per PRP gotcha (pyright --strict narrowstuple[Literal[...], ...]poorly); Pydantic still preserves Literal narrowing at construction.Test plan
uv run ruff check app/features/featuresets/— cleanuv run ruff format --check app/features/featuresets/— cleanuv run mypy app/— 188 source files, 0 errorsuv run pyright app/features/featuresets/— 0 errors, 0 warningsuv run pytest app/features/featuresets/tests/test_schemas.py -v— 46 passeduv run pytest app/features/featuresets/ -m "not integration" -v— 75 passed (no regression)uv run pytest app/features/forecasting/ app/features/registry/ -m "not integration"— 188 passed (downstreamconfig_hashconsumers safe)Follow-ups (out of scope here)