feat(launchpad): one-document one-task corpus manifest (#626) - #1558
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>
tucktuck101
left a comment
There was a problem hiding this comment.
Review — one-document one-task corpus manifest (#626)
Structurally sound, deterministic ordering is real, and the duplicate/limit checks are
genuine. The gap is input validation: build_manifest is presence-only, so several
plausible hand-authored mistakes pass silently. All four reproduced against this head:
audiences="agent" -> ('a','g','e','n','t')
blockers="#607" -> ('#','6','0','7')
misspelled keys (parnet_feature, tempalte) -> accepted, silently discarded
priority="banana", path="Not_A_Kebab_Path.TXT", filename mismatching path -> all accepted
High — tuple() on a string explodes a scalar into characters
manifest.py:117-121 does tuple(entry["blockers"]), tuple(entry["audiences"]),
tuple(entry["source_start_points"]). A scalar where a list belongs is the most likely
hand-authored plan mistake, and it contradicts build_manifest's own docstring promise
(:162-166) of raising "rather than dropping or silently coercing the bad row". Downstream,
#627 would render four blockers and five audiences from the strings above. One shared
_sequence(entry, key) helper that rejects str closes it.
Medium — unknown keys are silently discarded
missing = _REQUIRED_KEYS - entry.keys() (:101-107) checks only for absence. A row carrying
parnet_feature="#999" next to a correct parent_feature builds without a word, and the
typo vanishes. Also raise on entry.keys() - _REQUIRED_KEYS, naming the unexpected keys.
Medium — no field value is constrained
priority="banana", effort=-5, start_date="not-a-date", template="",
parent_feature=None and path="Not_A_Kebab_Path.TXT" are all accepted. The path case
matters most: launchpad/docs/corpus/schema/node.schema.json pins node ids to
^[a-z0-9]+(-[a-z0-9]+)*$, never renamed — so that row is a GitHub task to author a document
that cannot satisfy the schema. To be precise about scope, the schema constrains the node's
id, not the manifest's path, and the manifest carries no id; this is a gap between two
layers rather than a direct contradiction. A basename check
(^[a-z0-9]+(-[a-z0-9]+)*\.md$) closes it cheaply.
Medium — filename and path are carried independently and never cross-checked
path=".../capabilities/chat.md" with filename="totally-different.md" is accepted. Both
fields are required by #626's DoD and both reach the #627 consumer, so two sources of truth can
disagree about which file the task creates and nothing downstream can tell which is right.
Derive filename from path, or raise when they differ. (#1559's scaffold.py inherits this:
it derives the node id from filename while writing to path.)
Low — the 100-child Feature limit counts only this run
_check_feature_child_limits (:147-157) counts rows in the manifest under construction, so a
Feature that already owns 60 children on GitHub plus 50 new rows passes while the real parent
exceeds the limit. #626's DoD states the constraint about the Feature, not about one run. The
module has no GitHub access by design, so the fix is a parameter (existing_children: dict[str, int]), not a fetch.
What is correct
- Deterministic sort by document path, and duplicate
path/issue_titlerejection — both
real, both tested non-vacuously. ManifestValidationErroron a missing required field works exactly as documented.- Suite green (
Ran 10 tests ... OK). - This PR is the base of the #1559 and #1560 stack; both are stacked on
56b694426and carry
byte-identical copies of this file (verified withcmp), so it merges first and cleanly. - CI green at head (latest run per check).
Requesting changes for the string-explosion issue and the unchecked filename/path pair —
both are cheap and both propagate into two dependent PRs.
Reviewed by tucktuck101's review lane. Every behaviour above was reproduced by me against
this PR head before posting.
Summary
Adds
manifest.py:build_manifest(plan)validates a caller-supplied list ofplanned corpus documents into a deterministic manifest, enforcing that no
document is assigned to two tasks, no task owns two documents, and no
Feature exceeds GitHub's 100-sub-issue limit.
Related issue
Closes #626
Issue type
Task
Agent provenance
Objective
launchpad/project-intelligence/corpus/manifest.py, the one-documentone-task corpus manifest generator issue #626 asks for.
Impacted components
launchpad/project-intelligence/corpus/manifest.py
launchpad/project-intelligence/corpus/tests/test_manifest.py
Approach and rejected alternatives
build_manifest(plan)takes a caller-supplied list of dicts (one perplanned document) and returns a
Manifestof validatedManifestRowobjects, sorted by path for deterministic output. It enforces the DoD's
three structural rules by raising
ManifestValidationErrornaming theviolation, rather than dropping or silently coercing a bad row.
Rejected: having this module also decide WHICH documents the corpus needs,
their titles, or their templates. Rejected because that is real product
knowledge that lives in
launchpad/docs/corpus/AGENTS.md's per-typestandards/templates (issue #605, most still unmerged) and in whoever curates
the actual plan -- fabricating ~hundreds of real document records here would
be inventing a project decision this task was never scoped to make (#626's
own "Impacted components" list is only
manifest.pyandtests/, not acorpus-wide plan data file). This module owns the structural contract, not
the content.
Rejected: importing
inventory.py(#624) directly to auto-populatesource_start_points. Rejected to keepmanifest.pytestable in isolationand because the actual document-to-source mapping is a curation decision,
not something
inventory.py's raw item list determines on its own --source_start_pointsstays plain caller-supplied strings, which a futurecaller (a human, or the corpus-plan skill in #628) can populate FROM
inventory.py's output without this module depending on it.
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, 89/89, no failures. 10 of the 89 are this PR's new manifest tests.)Not verified
Did not verify this manifest schema against the actual
issue_plan.pyhelper (#627), because that helper does not exist yet -- "task metadata can
be consumed by the issue-plan helper without manual rewriting" is verified
structurally (flat dict, no nesting requiring reshaping) but not against a
real consumer. Did not exercise a plan with thousands of rows for
performance; the child-limit check is O(n) but was only tested at n=100/101.
Security implications
None. Pure in-memory validation of caller-supplied data; no file I/O, no
network access, no secrets involved.
Escalations
None. The task's structural requirements were concrete; the one design
choice (loose coupling from inventory.py, see rejected alternatives) follows
directly from #626's stated scope rather than requiring a new decision.