feat(lifecycle): consume narrative firmographics [LTV-Po.1]#130
Merged
Conversation
First half of the split LTV-Po (narrative-wiring; the recipe + e2e is Po.2). Per the locked decision, the recipe narrative DRIVES the lifecycle population's firmographics rather than them staying scheme-internal. - build_customer_population gains a narrative parameter. When provided, the account firmographic vocabularies come from narrative.market.icp_industries / narrative.market.geographies; when None, they fall back to the built-in procurement-ICP defaults (_ICP_INDUSTRIES / _GEOGRAPHIES). - _generate_accounts takes the resolved vocabularies (keyword-only, defaulting to the built-ins) and draws from them. - build_world threads its narrative through to the population builder. - Empty narrative vocabularies are rejected with a clear ValueError. Byte-identity: the no-narrative path passes the SAME built-in tuples, so the RNG draws — and every resulting bundle file — are unchanged. Verified vs main (both exposure modes, full SHA-256 of every file) using a fresh worktree. Band-weight distributions (employee/revenue/process-maturity) stay as module constants — they are not part of the narrative MarketSpec schema. Tests: narrative-driven industries/regions; no-narrative built-in fallback; empty-vocab rejection; narrative-population determinism. Full suite 1881 passed / 51 skipped; ruff + mypy clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Pull request overview
This PR wires the lifecycle scheme’s customer population firmographics (industries/regions) to the recipe narrative’s market spec, so recipe narratives become the source of truth for firmographic vocabularies while preserving byte-identity when no narrative is provided.
Changes:
- Add
narrative: NarrativeSpec | Nonetobuild_customer_population, validating non-emptymarket.icp_industries/market.geographies, and thread resolved vocabularies into_generate_accounts. - Thread
narrativethroughLifecycleScheme.build_worldinto the population builder. - Add focused tests for narrative-driven firmographics + determinism, and update roadmap/plan docs to reflect the Po.1/Po.2 split.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
leadforge/schemes/lifecycle/population.py |
Accept narrative-driven firmographic vocabularies; enforce non-empty narrative vocab; pass vocabularies into account generation. |
leadforge/schemes/lifecycle/__init__.py |
Thread narrative into population builder; update lifecycle scheme docstring. |
tests/schemes/lifecycle/test_population_narrative.py |
New tests covering narrative vocab usage, builtin fallback, empty-vocab rejection, and determinism. |
docs/ltv/roadmap.md |
Document LTV-Po split and mark Po.1 complete with narrative-driven firmographics. |
.agent-plan.md |
Update project plan notes to reflect Po split and locked decisions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
178
to
184
| n_accounts: int | None = None, | ||
| observation_date: str | None = None, | ||
| acquisition_window_weeks: int = _DEFAULT_ACQUISITION_WINDOW_WEEKS, | ||
| narrative: NarrativeSpec | None = None, | ||
| ) -> CustomerPopulationResult: | ||
| """Generate accounts and lifecycle customers with their latent states. | ||
|
|
Comment on lines
178
to
184
| n_accounts: int | None = None, | ||
| observation_date: str | None = None, | ||
| acquisition_window_weeks: int = _DEFAULT_ACQUISITION_WINDOW_WEEKS, | ||
| narrative: NarrativeSpec | None = None, | ||
| ) -> CustomerPopulationResult: | ||
| """Generate accounts and lifecycle customers with their latent states. | ||
|
|
Comment on lines
+57
to
+59
| ``narrative``, when provided, drives the population's firmographic | ||
| vocabularies (``market.icp_industries`` / ``market.geographies``); a | ||
| ``None`` narrative falls back to the built-in procurement-ICP defaults. |
…Po.1] Self-review of the narrative-firmographics wiring: Po.1 makes narrative.market.icp_industries / geographies drive the public industry / region columns, so a single-value vocabulary would yield a zero-variance firmographic feature in student_public (invariant #6). Not a Po.1 code defect (it correctly draws from whatever vocab it's given, and a single-value vocab is a legitimate request the validator should catch), but an easy-to-miss constraint on the Po.2 recipe narrative.yaml — flagged in the roadmap with a test to add (both columns ≥2 distinct values in the public bundle). Docs only. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
pr-agent-context report: This run includes unresolved review comments on PR #130 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.
# Copilot Comments
## COPILOT-1
Location: leadforge/schemes/lifecycle/population.py:184
URL: https://github.com/leadforge-dev/leadforge/pull/130#discussion_r3431442092
Root author: copilot-pull-request-reviewer
Comment:
The docstring’s determinism contract omits the new `narrative` input, but the generated population now depends on narrative-driven firmographic vocabularies. Update the “fully deterministic for a given …” tuple to include `narrative`.
## COPILOT-2
Location: leadforge/schemes/lifecycle/population.py:184
URL: https://github.com/leadforge-dev/leadforge/pull/130#discussion_r3431442145
Root author: copilot-pull-request-reviewer
Comment:
`build_customer_population` gained the `narrative` parameter, but the Args section doesn’t document it (including the non-empty vocabulary requirement). Adding a brief entry here will make the public API contract clearer.
## COPILOT-3
Location: leadforge/schemes/lifecycle/__init__.py:59
URL: https://github.com/leadforge-dev/leadforge/pull/130#discussion_r3431442167
Root author: copilot-pull-request-reviewer
Comment:
`LifecycleScheme.build_world` requires a `narrative` (Generator raises if it’s missing), so the docstring’s “when provided” / “None narrative falls back …” wording is misleading here. Consider describing the behavior without implying `None` is a supported input for this entry point.Run metadata: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
First half of the split
LTV-Po(narrative-wiring; the recipe assets + end-to-end round-trip are Po.2). Per the locked decision, the recipe narrative drives the lifecycle population's firmographics rather than staying scheme-internal.Change
build_customer_population(narrative=…)— when a narrative is provided, account firmographic vocabularies come fromnarrative.market.icp_industries/narrative.market.geographies; whenNone, they fall back to the built-in procurement-ICP defaults (_ICP_INDUSTRIES/_GEOGRAPHIES)._generate_accountstakes the resolved vocabularies (keyword-only, defaulting to the built-ins) and draws from them.build_worldthreads itsnarrativethrough to the population builder.ValueError.Band-weight distributions (employee / revenue / process-maturity) stay as module constants — they're not part of the narrative
MarketSpecschema.Byte-identity
The
narrative=Nonepath passes the same built-in tuples to_generate_accounts, so the RNG draws — and every resulting bundle file — are unchanged. Verified vsmain(both exposure modes, full SHA-256 of every file) via a fresh worktree.Tests (4 new; full suite 1881 passed / 51 skipped)
Narrative-driven industries/regions; no-narrative built-in fallback; empty-vocab rejection; narrative-population determinism.
Next: Po.2 — the
b2b_saas_ltv_v1recipe YAMLs (incl.narrative.yaml+difficulty_profiles.yaml), registry registration,difficulty_paramsresolution inbuild_world, and theGenerator.from_recipe("b2b_saas_ltv_v1").generate()round-trip — which completes M6.🤖 Generated with Claude Code