feat: adds stage aware schema changes and module changes - #7
Draft
jpower432 wants to merge 4 commits into
Draft
Conversation
Reframe the PRD model around jobs-to-be-done: job executors replace personas, desired-outcomes replace KPIs, journeys replace workflows. Add stage-aware constraints (ACs required only at Ready, FR `satisfies` required at Ready) and an in-file FR/journey coverage floor enforced by `cue vet`. Add valid/invalid conformance testdata. Assisted-by: Claude Code <noreply@anthropic.com>
Split dispatch by PRD state and invocation: a fresh Draft gets a `cue vet` + Guard review; `--full` runs the 5-agent council and is the only pass that gates Draft to Ready, mutating state.status only on author confirmation. Ready and later trigger no gating dispatch. Assisted-by: Claude Code <noreply@anthropic.com>
Add the lola-eval behavioral harness (config, provisioner, taskfiles) with the case-001 schema baseline, plus a CUE conformance test and CI workflow. Harness scaffolding adapted from lola-mod-review-council. Assisted-by: Claude Code <noreply@anthropic.com>
Add the discovery skill (researcher + gatherer + distiller) and its `/discovery` command for Stage-0 problem exploration before FRs exist: distill deduplicated open-questions into an existing PRD, or seed a parent PRD (framing, desired-outcomes, open-questions; no FRs) from a bare brief. Includes the discovery-tracker example, the case-002 discovery eval, and discovery docs. No schema changes: builds on the schema base. Assisted-by: Claude Code <noreply@anthropic.com>
jpower432
force-pushed
the
feat/discovery
branch
from
August 28, 2026 12:55
3d24fe4 to
459bd95
Compare
jflowers
requested changes
Sep 1, 2026
jflowers
left a comment
Contributor
There was a problem hiding this comment.
Review Council Report
Reviewers: 9 Divisor agents (Adversary, Architect, Curator, Guard, SRE, Testing, Envoy, Herald, Scribe)
Pre-flight: CI PASS (task test green on GitHub Actions)
Walkthrough
| Directory | Files | Summary | Focus |
|---|---|---|---|
.github/workflows/ |
2 | CI workflow updates | ci-cd |
.lola-eval/ |
12 | New eval framework | standard |
.taskfiles/ |
2 | Task definitions + scripts | standard |
module/agents/ |
6 | PRD agent updates + 3 new discovery agents | documentation |
module/commands/ |
2 | Updated prd-review + new discovery command | documentation |
module/skills/discovery/ |
2 | New discovery skill + protocol | documentation |
module/skills/prd-review/ |
2 | Updated prd-review skill + protocol | documentation |
examples/ |
3 | Example PRDs updated | documentation |
testdata/invalid/ |
7 | New invalid test fixtures | test-quality |
testdata/valid/ |
2 | New valid test fixtures | test-quality |
| root | 7 | prd.cue, Taskfile, AGENTS.md, README.md, etc. | standard |
HIGH — 7 findings
| # | Finding | Reporters |
|---|---|---|
| H1 | No migration guidance for breaking schema changes — 6+ field renames (persona→executor, workflow→journey, kpis→desired-outcomes, snake_case→kebab-case), removed concepts (personas, kpis), and Review→Ready status rename. Existing PRDs will silently fail cue vet with no explanation or migration path. |
Envoy, Herald, Curator, SRE, Guard |
| H2 | Missing CHANGELOG.md — Breaking schema changes, new discovery stage, two-tier review model, and NOTICE file addition all require changelog entries. | Curator, SRE, Herald |
| H3 | No docs issues filed — Three distinct user-facing changes (discovery stage, schema migration, two-tier review) need documentation tracking issues. | Curator |
| H4 | --full flag behavioral change undocumented — Default /prd-review silently changed from 5-agent council to Guard-only. Existing users get reduced review with no warning. |
Guard |
| H5 | Shell test runner lacks assertion specificity — task test validates pass/fail only, not which constraint caused rejection. A regression in one CUE constraint could be masked by another. |
Testing |
| H6 | Missing test fixtures for maturity gate and removed constructs — No invalid fixture for Ready AC requirement (ready-fr-missing-ac.yaml). No regression fixtures for removed personas on parent or workflow on phase. |
Testing |
| H7 | sed -i macOS incompatibility — provision.sh uses GNU sed -i syntax that fails on macOS (BSD sed requires backup extension). Developers on macOS cannot run task lola-eval:provision. |
SRE, Adversary |
MEDIUM — 10 findings
| # | Finding | Reporters |
|---|---|---|
| M1 | publish-cue.yml removes validation without replacement — Inline cue vet loop removed; publish workflow can now publish unvalidated schema to the CUE Central Registry. |
Architect, SRE |
| M2 | DRY violation: prd.cue duplicated in 2 eval starters — Byte-identical 129-line copies. Schema changes require updating 3 files. |
Architect |
| M3 | CUE version pinned in CI (v0.17.0) but not locally — Taskfile relies on whatever cue is on PATH. CUE 0.17 features may behave differently on older versions. |
SRE |
| M4 | provision.sh lacks cleanup on failure — Exits on error but leaves partially provisioned starters, producing misleading eval results on retry. |
SRE |
| M5 | Inconsistent task_version across eval cases — case-001 uses "2", case-002 uses "1". Undocumented version difference. |
Architect, Testing |
| M6 | No spec artifacts for 49-file PR — Constitution and AGENTS.md require spec workflow for non-trivial changes. | Guard |
| M7 | Hardcoded example list in vet-examples — Three files explicitly listed; new examples won't be validated. Old publish-cue.yml used a glob. |
SRE, Testing |
| M8 | README prose density — Discovery and prd-review bullets pack 5+ concepts into single sentences. Agent roster table removed. | Scribe, Envoy, Herald |
| M9 | Schema/agent enforcement split undocumented at schema level — desired-outcomes optional in CUE but Guard blocks at Ready/Approved. Intentional but not discoverable from the schema. |
Architect |
| M10 | test task uses set -u without set -e — Unexpected failures within the loop could be silently swallowed. |
SRE |
LOW — 12 findings
| # | Finding |
|---|---|
| L1 | Stale open_question reference in prd-curator.md (missed rename to open-questions) |
| L2 | Missing EOF newline in ci.yml |
| L3 | Trailing whitespace in discovery-gatherer.md and discovery-researcher.md |
| L4 | Floating @main git dep for lola-eval (documented trade-off, dev-only) |
| L5 | curl | sh suggestion in error message text (not executed) |
| L6 | Task version floating 3.x in CI |
| L7 | Heredoc tab indentation fragile in ensure-lola-eval.sh |
| L8 | cue vet argument order inconsistency in discovery SKILL.md vs rest of repo |
| L9 | Missing language identifiers on README code blocks |
| L10 | Ready→Approved prohibition repeated 3x in same SKILL.md file |
| L11 | Discovery command usage line ambiguous (<parent.yaml | "problem brief">) |
| L12 | Sentence reflow in prd-guard.md after rename edit |
Positive Observations
- SHA-pinned CI actions with version comments, least-privilege permissions,
persist-credentials: false - No secrets, credentials, or injection vectors in any changed file
- Clean eval isolation with synthetic
.gitconfigand null'dGIT_CONFIG_GLOBAL - NOTICE file properly follows Apache-2.0 Section 4(c)/(d)
- CUE schema inline comments are thorough — explain WHAT, HOW, and WHY
- Rename completeness is impressive across 49 files with only 1 miss
- Usage examples in README effectively demonstrate both modes
- Maturity-gated schema is a well-designed graduated enforcement model
Verdict
| Reviewer | Verdict |
|---|---|
| Adversary | APPROVE |
| Architect | APPROVE |
| Curator | REQUEST CHANGES |
| Guard | REQUEST CHANGES |
| SRE | REQUEST CHANGES |
| Testing | REQUEST CHANGES |
| Envoy | NEEDS REVISION |
| Herald | NEEDS REVISION |
| Scribe | REQUEST CHANGES |
Council Verdict: REQUEST CHANGES (7 of 9 reviewers)
Required Changes Before Merge
- H1 + H2: Add
CHANGELOG.mdwith migration mapping (old→new field names, removed fields, status enum change). Consider asedrecipe for bulk PRD migration. - H3: File 3 documentation issues (
docs:label) for discovery stage, schema migration guide, and two-tier review behavior. - H4: Add prominent migration/breaking-change note about default
/prd-reviewbehavior change. - H5 + H6: Add assertion specificity to test runner (or document why pass/fail-only is acceptable) and add missing test fixtures (
ready-fr-missing-ac.yaml,parent-has-personas.yaml,phase-has-workflow.yaml). - H7: Fix
sed -iportability inprovision.shfor macOS compatibility. - L1: Fix stale
open_question→open-questionsinprd-curator.md. - L2: Add trailing newline to
ci.yml.
Recommended Improvements (Non-blocking)
- M1: Add
task vet-examplestopublish-cue.ymlbefore publish step - M2: Have
provision.shcopy rootprd.cueinto starters (eliminate duplication) - M7: Change
vet-examplesto use glob instead of hardcoded file list - M8: Improve README prose density — break dense bullets into sub-lists
- M9: Add CUE comment on
desired-outcomes?:explaining the schema/agent enforcement split
Collaborator
Author
|
Thanks for the review @jflowers. This PR needs a rebase so some of the request for changes may be from an already merged PR. I will address these and bootstrapping |
jpower432
added a commit
that referenced
this pull request
Sep 3, 2026
* fix: address PR #7 review council findings Migration guidance (CHANGELOG), missing removed-field test fixtures (personas, workflow), macOS-safe sed in provision.sh, schema validation gate on publish-cue.yml, specific cue vet error output in task test, globbed vet-examples, and stale open_question rename in prd-curator.md. Assisted-by: Claude Code <noreply@anthropic.com> fix: address review council findings on prior commit - provision.sh: fix ERR trap referencing stale/unset $clean across loop iterations (could rm -rf an unrelated already-finished case); also clean up .gitconfig and clear the trap after the loop - Taskfile.yml: drop sources:-based caching on vet-examples — it was silently skipping re-validation on unchanged example files even after schema edits, defeating the task's purpose - CHANGELOG.md: clarify the breaking schema change already shipped in #8, this entry backfills documentation rather than describing a pending change - prd-curator.md: fix subject/verb mismatch from the open-questions rename ("open-questions entry", not bare plural as subject) Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Jennifer Power <barnabei.jennifer@gmail.com> * chore: boostrap unbound-force Signed-off-by: Jennifer Power <barnabei.jennifer@gmail.com> * chore: update .gitignore Signed-off-by: Jennifer Power <barnabei.jennifer@gmail.com> Co-authored-by: Jay Flowers <jay.flowers@gmail.com> --------- Signed-off-by: Jennifer Power <barnabei.jennifer@gmail.com> Co-authored-by: Jay Flowers <jay.flowers@gmail.com>
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.
This PR builds on #8 by adds skills, commands, and agents for the first stage.
--fullis run the entire Review Council is run to evaluate readiness