feat(launchpad): one-node corpus scaffold helper (#632) - #1559
Conversation
build_manifest(plan) validates a caller-supplied list of planned corpus documents into a deterministic Manifest: every row carries path, filename, issue title, parent feature, priority, dates, effort, blockers, template, purpose, audiences and source start points. Rejects a document assigned to two tasks, a task owning two documents, and any Feature exceeding GitHub's 100-sub-issue limit. Curating the actual plan content is out of scope -- this module only enforces #626's structural guarantees on whatever plan it is given. Signed-off-by: Serina Mcfall <serina.mcfall@gmail.com>
scaffold_node(root, row, node_type=, origin=, revision=) creates exactly one corpus file from a #626 manifest row. type/origin/status/ audiences are validated against node.schema.json's real enums (read at call time, never hardcoded), and the template is checked against the real launchpad/docs/corpus/templates/ registry -- empty today, so every call correctly fails closed until #605 lands templates there. The one evidence entry this module writes is the mechanical provenance/revision citation AGENTS.md describes; all substantive evidence is left for the corpus-author skill (#629). mode="create" (default) refuses an existing file; mode="update" requires one. Signed-off-by: Serina Mcfall <serina.mcfall@gmail.com>
tucktuck101
left a comment
There was a problem hiding this comment.
Review — one-node corpus scaffold helper (#632)
Sound module, tests run green (41 passed), and the schema-driven design is the right shape.
Three gaps, one of which lets the module write a file the repo's own validator rejects.
High — revision is interpolated into a commit citation without validation
scaffold.py:145 writes f"commit {revision}" into the node's provenance evidence entry, and
revision is the one input the module never checks — every other input is validated against
the loaded schema and fails closed.
The repo's validator requires
_COMMIT_CITATION_RE = re.compile(r"^commit\s+[0-9a-fA-F]{7,40}\b")
(launchpad/project-intelligence/corpus/validate.py:565, on the integration branch;
failure message at :758). So revision="HEAD", a tag like v1.2.3, or a short SHA under 7
characters each produce a node written to disk that fails corpus-validate with "matches none
of CONTRACT.md's six supported citation forms". A 40-hex value passes.
That is a fail-open in a module whose stated contract is that nothing falls through
unvalidated. One check closes it: ^[0-9a-fA-F]{7,40}$ -> ScaffoldError.
Medium — the id pattern is hardcoded, contradicting the docstring two lines above it
scaffold.py:48 is _ID_PATTERN = re.compile(r"^[a-z0-9]+(-[a-z0-9]+)*$"), but the module
docstring at :16 says the id is "validated against the schema's own kebab-case pattern",
immediately after asserting that enums are "read from the file at call time, not hardcoded
here, so this module can never silently drift from the schema it is scaffolding against". The
loaded schema is consumed only for type/origin/status/audiences (:97-100).
It matches node.schema.json's properties.id.pattern today, and nothing keeps the two in
step — the exact drift vector the surrounding design exists to prevent. Read
schema["properties"]["id"]["pattern"].
Medium — nothing cross-checks row.filename against basename(row.path)
node_id derives from row.filename (:125) while the file is written to root / row.path
(:133). manifest.py does not enforce agreement either, so a row with
filename="totally-different.md" and path=".../capabilities/chat.md" writes
capabilities/chat.md carrying id: totally-different, and the validator accepts it. Corpus
ids are immutable once assigned, so that is a durable inconsistency no downstream check
catches. One guard: if Path(row.path).name != row.filename: raise ScaffoldError(...).
Low — shared corpus_manifest module registration
scaffold.py:42-46 loads manifest.py under sys.modules.setdefault("corpus_manifest", ...)
while #1560's tests/test_issue_plan.py:22-24 assigns sys.modules["corpus_manifest"]
outright. In a merged tree manifest.py executes twice and two distinct ManifestRow classes
coexist. Harmless today — neither module does an isinstance check, and the merged state runs
green — but it is the one latent collision surface between these two stacked branches. A
shared loader helper removes it. Note setdefault prevents the second registration, not the
second execution.
What is correct
- Reading the closed enums from
node.schema.jsonat call time rather than restating them is
the right call and is what keeps this module honest as the schema evolves. - Tests pass and are not vacuous for the paths they cover.
- Placement, frontmatter shape and generated evidence structure match the merged corpus nodes.
- No conflict with #1560: both branches are stacked on
56b694426(#1558) and their
manifest.pycopies are byte-identical — verified withcmp. Merge order is #1558 first,
then these two in either order. - CI green at head (latest run per check).
Reviewed by tucktuck101's review lane. Every finding above was reproduced against the PR head
before posting.
…lution Blocking finding from tucktuck101's review confirmed and fixed: step 5 called scaffold.scaffold_node(...) unconditionally, with no branch. Verified against this PR's own head: neither launchpad/docs/corpus/templates/ nor launchpad/project-intelligence/corpus/scaffold.py exists yet, on this branch or on launchpad -- scaffold.py arrives with #1559, unmerged. Once it does land, _known_templates(root) returns an empty frozenset while templates/ is missing, so scaffold_node raises ScaffoldError on every call, and step 8 already tells the agent to treat ScaffoldError as ground truth to stop on. So an agent following steps 1-4 correctly hit a hard stop with no instruction for step 4's "Absent altogether" branch -- the skill's own headline contribution to resolving the template state this corpus is actually in. Step 5 now branches exactly the way step 4 already does: scaffold when the template is merged or provisional, otherwise hand-author the frontmatter directly against node.schema.json (writing the same provenance evidence entry scaffold_node would have) and say so in the node's scope section, per AGENTS.md step 8. Not fixed here (High/Medium, not blocking, reported separately): the node_type/origin literal ellipses with no derivation rule, and step 1's "corpus-plan's ledger" not actually containing the fields it asks for. No automated suite covers SKILL.md prose; verification stamp touched per the hook's documented no-suite fallback, not earned by a test run. Signed-off-by: Serina Mcfall <serina.mcfall@gmail.com>
Summary
Adds
scaffold.py:scaffold_nodecreates exactly one corpus file from a#626 manifest row, with front matter validated against
node.schema.json'sreal enums and the real (currently empty)
templates/registry -- neverinvented values, always fails closed on anything unrecognised.
Related issue
Closes #632
Issue type
Task
Agent provenance
Objective
launchpad/project-intelligence/corpus/scaffold.py, the one-node corpus scaffold helper issue #632 asks for.Impacted components
launchpad/project-intelligence/corpus/scaffold.py
launchpad/project-intelligence/corpus/tests/test_scaffold.py
Note on this diff's base: this branch is built on top of
task/626-corpus-manifest(#626, PR #1558, still open) becausescaffold_nodegenuinely needsmanifest.ManifestRowto exist. Until #1558 merges, this PR's diff also showsmanifest.py/test_manifest.pyfrom that PR -- same shape as the corpus batches' own "base off the current head of the real dependency, not launchpad, until it merges" convention. Nothing in this PR's own two files touches manifest.py.Approach and rejected alternatives
scaffold_nodereadsnode.schema.jsonat call time and validatesnode_type/origin/status/audiencesagainst its real enums, so thismodule can never silently drift from the schema it targets. The template
registry is the real
launchpad/docs/corpus/templates/*.mddirectorylisting -- empty in this repository today, so every scaffold call correctly
raises "unknown template" until issue #605 lands templates there, with no
change needed to this module when that happens. The one
evidenceentrythis module writes is the provenance/revision citation
docs/corpus/AGENTS.md's "Creating a node" step 6 calls mechanical; everyother evidence entry is left for the corpus-author skill (#629).
Rejected: hardcoding the list of known templates (capability, component,
concept, ...). Rejected because that list is real project content still
landing via issue #605's still-open template PRs -- hardcoding it here would
drift the moment a template is renamed or a new one is added, and would
require editing this module every time, which the real-registry approach
avoids entirely.
Rejected: having the caller supply
node_typepre-validated. Rejectedbecause #632's DoD says "front matter is populated from manifest/schema
values" -- validating against the actual schema enum (not trusting the
caller) is what makes this module's front matter genuinely schema-derived
rather than merely schema-shaped by convention.
Rejected: writing
evidence: []for a schema-technically-incomplete stub.Rejected because
node.schema.jsonrequiresminItems: 1onevidence,and AGENTS.md already describes a mechanical, non-authorial evidence entry
(the revision citation) that satisfies it honestly, without inventing any
subject-matter claim -- see
test_scaffolded_node_actually_passes_the_real_validator.Verification
Command run:
Raw output:
(The
FAILline is validate.py's own diagnostic output from a test thatdeliberately exercises a nonexistent-root path -- the suite's actual result
is
OK, 105/105, no failures. 16 of the 105 are this PR's new scaffoldtests, including one that runs the real
validate.pyover a scaffoldednode and asserts zero errors.)
Not verified
Did not test
mode="update"against a file whose existing front matter hasalready been hand-edited by an author (e.g. real evidence entries added) --
scaffold_nodein update mode fully overwrites the file, including anyevidence beyond the provenance entry, which is destructive if called after
authoring has started. Nothing in this module warns about that; it is
implicit from "explicit update mode" in the DoD, not verified against a
realistic authored-then-rescaffolded scenario. Did not verify behavior when
node.schema.jsonitself is malformed or missing required schema keys thismodule assumes exist (
properties.type.enum, etc.) -- it would raise aKeyError, not aScaffoldError, which is a real inconsistency with thismodule's own "always fails closed with ScaffoldError" framing.
Security implications
None new. Read-only against the schema/template registry, writes only to
the one path the manifest row + corpus-root prefix check authorize; the
schema.py/schema/-exclusion check prevents writing into schema/'s
deliberately-unvalidated subtree.
Escalations
Whether
mode="update"should refuse to overwrite a file that already hasmore than the one provenance evidence entry (i.e. one an author has started
working on) -- raised in "Not verified" above, not decided here. #632's DoD
only asks for "existing files are never overwritten without an explicit
update mode," which this satisfies literally; whether that's sufficient
protection once real authoring begins is a judgment call for whoever
integrates this with #629.