feat(cli): add vouch new scaffold command#333
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds a Changesvouch new command
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant CLI as new_cmd
participant Registry as page_kind_registry
participant Proposals as propose_page/propose_entity
User->>CLI: vouch new <kind> [--field ...] [--dry-run] [--json]
CLI->>Registry: resolve(kind)
Registry-->>CLI: required_fields, schema, required_citations
CLI->>CLI: stub/prompt missing required fields
alt dry-run
CLI-->>User: print draft / JSON
else create
CLI->>Proposals: propose_page(...) or propose_entity(...)
Proposals-->>CLI: proposal_id
CLI-->>User: print result / JSON
end
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
tests/test_new_scaffold.py (2)
1-158: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueTest file name doesn't mirror the module under test.
This file exercises the
newcommand defined insrc/vouch/cli.py, not anew_scaffoldmodule. Per coding guidelines, test filenames should mirror module names astests/test_<module>.py— these cases likely belong intests/test_cli.py(or the file should be renamed to reflectcli).As per coding guidelines, "Test file names must mirror module names using the
tests/test_<module>.pyconvention."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_new_scaffold.py` around lines 1 - 158, The test file name does not match the module being exercised; these tests target the cli entrypoint in vouch.cli, not a new_scaffold module. Rename or relocate the suite to follow the tests/test_<module>.py convention, and keep the existing test functions (such as test_new_decision_page_stubs_frontmatter and test_new_pending_not_approved) under the cli-focused test file name.Source: Coding guidelines
32-158: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNo coverage for
--interactiveprompting path.The PR objectives call out
--interactivesupport for missing fields, but none of the added tests exercise it (e.g. viaCliRunner(input=...)). Consider adding a case that confirms prompted values land inpayload["metadata"].🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_new_scaffold.py` around lines 32 - 158, Add test coverage for the `--interactive` path in the `new` command, since the current cases in `test_new_scaffold.py` only cover direct flags and dry-run behavior. Create a `CliRunner` invocation with `input=...` for a kind with required fields, and assert the prompted answers are stored in `pr.payload["metadata"]` after `cli` runs. Use the existing `test_new_field_prefill_parsed_as_yaml` and `test_new_dry_run_writes_nothing` patterns as a guide, and keep the assertion focused on the interactive prompt flow for `new`.src/vouch/cli.py (1)
1033-1033: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMake the allowlist comment explain why it exists.
The current comment restates the constant instead of documenting the constraint.
As per coding guidelines,
src/**/*.py: “Do not use marketing tone like "we" or "let's" in code comments; comments should explain why, not what.”Proposed refactor
-# Entity kinds `vouch new` can scaffold (issue `#330`). +# Keep entity scaffolding intentionally narrow because `propose_entity` accepts arbitrary type strings.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/vouch/cli.py` at line 1033, The allowlist comment near the `vouch new` scaffold kinds is too descriptive and should explain the constraint instead of restating the constant. Update the comment attached to the entity kinds allowlist in `cli.py` so it states why only those kinds are permitted for `vouch new`, and avoid marketing-style wording; keep the guidance tied to the related `vouch new` scaffolding logic and the allowlist definition.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/vouch/cli.py`:
- Around line 1204-1206: The citation reminder path in propose_page appends
_CITATION_REMINDER even though the page is going to be rejected for
citation-required kinds without claims or sources. Move the citation gating
before filing so unfileable proposals are rejected first, or make the reminder
draft-only in the propose_page flow and any related submit path around
citation_reminder/body handling.
- Line 1029: The meta parsing and draft preview path is leaking YAML-related
failures: `_parse_meta()` and `_prompt_missing_fields()` should catch
`yaml.safe_load()` exceptions and rethrow them as `click.BadParameter` so
invalid frontmatter is reported cleanly. Update `_print_new_page_draft()` to
avoid `json.dumps()` on `frontmatter`, since YAML-native types like dates can
break the dry-run preview; use a serializer that safely handles YAML values.
Keep the fix localized to `_parse_meta()`, `_prompt_missing_fields()`, and
`_print_new_page_draft()` so malformed input and preview rendering both fail
gracefully.
In `@tests/test_new_scaffold.py`:
- Around line 143-150: The assertion in
test_new_required_citations_reminder_in_body is vacuous because it checks for a
literal token that never appears, so the OR always passes. Update the test to
assert against the actual rendered reminder text produced by the new command
output (via cli/new and _CITATION_REMINDER) and remove the always-true disjunct
so the test verifies the expected citations-required messaging in res.output.
---
Nitpick comments:
In `@src/vouch/cli.py`:
- Line 1033: The allowlist comment near the `vouch new` scaffold kinds is too
descriptive and should explain the constraint instead of restating the constant.
Update the comment attached to the entity kinds allowlist in `cli.py` so it
states why only those kinds are permitted for `vouch new`, and avoid
marketing-style wording; keep the guidance tied to the related `vouch new`
scaffolding logic and the allowlist definition.
In `@tests/test_new_scaffold.py`:
- Around line 1-158: The test file name does not match the module being
exercised; these tests target the cli entrypoint in vouch.cli, not a
new_scaffold module. Rename or relocate the suite to follow the
tests/test_<module>.py convention, and keep the existing test functions (such as
test_new_decision_page_stubs_frontmatter and test_new_pending_not_approved)
under the cli-focused test file name.
- Around line 32-158: Add test coverage for the `--interactive` path in the
`new` command, since the current cases in `test_new_scaffold.py` only cover
direct flags and dry-run behavior. Create a `CliRunner` invocation with
`input=...` for a kind with required fields, and assert the prompted answers are
stored in `pr.payload["metadata"]` after `cli` runs. Use the existing
`test_new_field_prefill_parsed_as_yaml` and `test_new_dry_run_writes_nothing`
patterns as a guide, and keep the assertion focused on the interactive prompt
flow for `new`.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 79c7c58a-790f-4d22-864f-dfec63976dad
📒 Files selected for processing (3)
CHANGELOG.mdsrc/vouch/cli.pytests/test_new_scaffold.py
There was a problem hiding this comment.
Pull request overview
This PR adds a new vouch new <kind> CLI command that scaffolds a typed page or entity proposal from the page-kind registry (issue #330). It is a client-side convenience that composes the existing propose_page / propose_entity calls, so it introduces no new kb.* surface and stays fully inside the review gate — a scaffolded draft lands only as a pending proposal that a human still approves. The command resolves the requested <kind> against load_page_kind_registry(store) (page kinds win on name collisions unless --entity is set), stubs required frontmatter fields, and supports --field key=value, --interactive, --body, --claim/--source, --dry-run, and --json.
Changes:
- New
newcommand plus helpers (_resolve_new_kind,_stub_page_frontmatter,_prompt_missing_fields, draft printers) and a_SCAFFOLD_ENTITY_TYPESallow-list;_parse_metageneralized to accept aflagname for diagnostics and reused for--field. - 11 new tests in
tests/test_new_scaffold.pycovering page/entity scaffolds, YAML--field, dry-run (no write), JSON output, collision dispatch, unknown-kind errors, and citation reminders. CHANGELOG.mdentry under[Unreleased] > Added.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/vouch/cli.py | Adds the vouch new command and supporting helpers; generalizes _parse_meta to be reused by --field. |
| tests/test_new_scaffold.py | New test module exercising the scaffold across page/entity, dry-run, JSON, collision, and citation cases. |
| CHANGELOG.md | Documents the new vouch new feature under the Unreleased/Added section. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
catch invalid yaml in --field and interactive prompts, gate citation-required kinds before filing, unify dry-run json id key, and move new command tests into test_cli.py. Co-authored-by: Cursor <cursoragent@cursor.com>
|
@copilot resolve the merge conflicts in this pull request |
scaffold typed page and entity proposals from the page-kind registry so authors get correctly-shaped drafts through the normal review gate. Co-authored-by: Cursor <cursoragent@cursor.com>
catch invalid yaml in --field and interactive prompts, gate citation-required kinds before filing, unify dry-run json id key, and move new command tests into test_cli.py. Co-authored-by: Cursor <cursoragent@cursor.com>
2b9ac97 to
079b94e
Compare
|
@plind-junior Resolved conflicts |
Summary
vouch new <kind>to scaffold typed page or entity proposals from the page-kind registry (feat: vouch new <kind> — scaffold a typed page/entity proposal from a template #330)--field,--interactive,--dry-run, and--json; page kinds win on name collisions unless--entityis setpropose_page/propose_entitycalls — no newkb.*surfaceTest plan
pytest tests/test_new_scaffold.py -q(11 tests)ruff check src/vouch/cli.py tests/test_new_scaffold.pyvouch new decision --title "pick X"creates a pending page proposalvouch new person --name alice-examplecreates a pending entity proposalvouch new meeting-notes --title Sync --dry-runprints missing fields without writingCloses #330
Summary by CodeRabbit
New Features
vouch newcommand to scaffold new page or entity proposals from configured kinds.--field key=value,--interactive,--dry-run, and--jsonfor more flexible proposal creation.Bug Fixes