refactor(render): scheme-agnostic build_manifest + schema v6 [LTV-Pn.1]#121
Merged
Conversation
First sub-PR of the split LTV-Pn (M6). Decouples the manifest builder from the lead-scoring scheme so the lifecycle scheme can reuse it, and records which scheme produced each bundle. - build_manifest no longer takes the lead-scoring `world_graph`. It takes `generation_scheme: str` (required), `motif_family: str | None = None`, and an `extra_fields` mapping for scheme-specific top-level keys (the lifecycle scheme will add `observation_date` / forward windows). A collision guard rejects extra_fields that would clobber a core key. - Every manifest now records `generation_scheme` (lead_scoring / lifecycle). - BUNDLE_SCHEMA_VERSION 5 -> 6 (history note added). The lead-scoring published *shape* is unchanged: tables/ and tasks/ parquet files are byte-identical to v5 (verified via pinned-timestamp SHA-256 across the full instructor bundle); only manifest.json changes (new field + version). - Removes the render.manifests -> lead_scoring.structure.graph TYPE_CHECKING back-reference (partial discharge of carried cleanup #3; the core.models.WorldBundle back-refs follow in Pn.2). - Schema-contract gate renamed test_bundle_schema_v5_contract.py -> _v6_, asserts version "6", and adds a generation_scheme assertion. The pinned column/table sets carry over verbatim (shape unchanged). Callers updated: lead_scoring.write_bundle passes generation_scheme=self.name, motif_family=world_graph.motif_family. Plan: docs/ltv/roadmap.md records the four-way LTV-Pn split (Pn.1…4) and marks the cleanup-#3 partial discharge. Tests: new tests/render/test_manifest_scheme_agnostic.py (generation_scheme recorded, motif_family default/passthrough, extra_fields merge + collision guard, required-arg); v6 contract test. Full suite 1801 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 decouples leadforge.render.manifests.build_manifest from the lead-scoring scheme so it can be reused by other generation schemes (notably the upcoming lifecycle pipeline), while bumping the bundle manifest schema version to v6 to record the producing scheme.
Changes:
- Refactors
build_manifestto be scheme-agnostic: removesworld_graph, adds requiredgeneration_scheme, optionalmotif_family, andextra_fieldswith collision protection. - Bumps
BUNDLE_SCHEMA_VERSIONfrom"5"→"6"and updates contract tests to assert v6 +generation_scheme. - Updates lead-scoring and render tests to pass the new
build_manifestarguments; documents the LTV-Pn split in roadmap/agent plan.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
leadforge/render/manifests.py |
Makes build_manifest scheme-agnostic; adds generation_scheme, motif_family, extra_fields; bumps schema version to v6. |
leadforge/schemes/lead_scoring/__init__.py |
Updates lead-scoring bundle writer to pass generation_scheme and motif_family into the manifest. |
tests/render/test_render.py |
Updates manifest construction in unit tests to match the new build_manifest signature. |
tests/render/test_manifest_scheme_agnostic.py |
Adds focused unit coverage for scheme-agnostic manifest behavior (required arg, defaults, merging, collision guard). |
tests/render/test_bundle_schema_v6_contract.py |
Updates the schema contract test to v6 and asserts generation_scheme is present and correct. |
tests/integration/test_snapshot_safe_bundle.py |
Updates docstring reference from v5 → v6 (noted inconsistency addressed in PR comment). |
docs/ltv/roadmap.md |
Documents the split of LTV-Pn into Pn.1–Pn.4 and describes this PR’s scope. |
.agent-plan.md |
Updates agent plan notes to reflect the LTV-Pn split and this PR as Pn.1. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…Pn.1] Self-review findings on the manifest-generalization PR. 1. Verification gap (mine): the byte-identity proof covered only the research_instructor bundle. Confirmed the student_public path — which takes the snapshot-safe relational route and also calls build_manifest — is unchanged and deterministic, and carries generation_scheme + v6. No code change. 2. The inspect CLI is the user-facing manifest viewer but ignored the new first-class generation_scheme field. Added a conditional "Scheme:" row (conditional so pre-v6 bundles still render without a "?" placeholder, matching the existing v3+/v4+ field handling). Also made the motif_family row null-safe: a lifecycle bundle records motif_family=null, which .get(..., '?') would render as the literal "None"; now shows "?". 3. Bumped four release/critique test fixtures from a hardcoded bundle_schema_version "5" to "6". They are inert (nothing compares them to the current version, which is why the suite stayed green), but a fixture claiming "5" while the package emits "6" is misleading. Tests: inspect-shows-generation-scheme CLI test; the header-order regression guard still holds (the new row sits between Recipe and Seed without reordering the pinned eight). Full suite 1802 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.
…[LTV-Pn.1] Copilot review on #121: the module docstring opening line still said "bundle schema v5" while line 9 (updated in this PR) references v6. Make the version consistent. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
pr-agent-context report: No unresolved review comments, failing checks, or actionable patch coverage gaps were found on PR #121 in repository https://github.com/leadforge-dev/leadforge. Treat this PR as all clear unless new signals appear.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 sub-PR of the split
LTV-Pn(M6). The originalLTV-Pnbundled envelope-generalization + 3 carried cleanups + config + task model + the lifecycle pipeline + schema bump — too large for one reviewable, byte-identity-guarded PR. It's now four sub-PRs (Pn.1–4) + the existing Po; this is Pn.1, the manifest decoupling.build_manifestwas coupled to the lead-scoring scheme (it required aworld_graph). This PR makes it scheme-agnostic so the lifecycle scheme can reuse it, and records which peer scheme produced each bundle.Changes
build_manifestsignature: dropsworld_graph; addsgeneration_scheme: str(required),motif_family: str | None = None, andextra_fields: dict | Nonefor scheme-specific top-level keys (lifecycle will addobservation_date/ forward windows). A collision guard rejectsextra_fieldsthat would clobber a core key.generation_scheme(lead_scoring/lifecycle).BUNDLE_SCHEMA_VERSION5 → 6. The lead-scoring published shape is unchanged —tables/andtasks/parquet are byte-identical to v5 (verified via pinned-timestamp SHA-256 across the full instructor bundle); onlymanifest.jsonchanges (new field + version label).render.manifests→lead_scoring.structure.graphTYPE_CHECKINGback-ref. Thecore.models.WorldBundleback-refs follow in Pn.2 onceWorldBundleholds scheme-agnostic artifacts.test_bundle_schema_v5_contract.py→_v6_, asserts"6", adds ageneration_schemeassertion; pinned column/table sets carry over verbatim (shape unchanged).Compatibility note
A consumer pinning
bundle_schema_version == "5"will need to accept"6". The data is byte-identical; only the manifest gained a field. Existing published v5 bundles are unaffected (heads-up already tracked in leadforge-datasets-private#8).Tests
tests/render/test_manifest_scheme_agnostic.py:generation_schemerecorded,motif_familydefault/passthrough,extra_fieldsmerge + collision guard, required-arg.Roadmap
docs/ltv/roadmap.mdrecords the four-wayLTV-Pnsplit and the cleanup-#3 partial discharge. Next: Pn.2 (scheme-agnosticWorldBundle+ exposure hook + shared bundle orchestrator).🤖 Generated with Claude Code