Skip to content

M12: CLI polish — surface v4 manifest fields, add --json, expose snapshot/task flags#60

Merged
shaypal5 merged 2 commits into
mainfrom
m12-cli-polish
May 5, 2026
Merged

M12: CLI polish — surface v4 manifest fields, add --json, expose snapshot/task flags#60
shaypal5 merged 2 commits into
mainfrom
m12-cli-polish

Conversation

@shaypal5

@shaypal5 shaypal5 commented May 5, 2026

Copy link
Copy Markdown
Contributor

Summary

Catches the user-facing CLI up to bundle schema v4 (which shipped today as #59) and clears two M12 deferrals from .agent-plan.md.

Before this PR, a user who downloaded a v4 bundle and ran leadforge inspect got no signal that the bundle is windowed, no idea which columns were redacted, no view of the primary task or label window — they had to cat manifest.json | jq. The mirror gap on leadforge generate was that --snapshot-day, --primary-task, and --label-window-days were only reachable via a YAML --override file even though Generator.from_recipe() already accepted all three.

What changed

(A) leadforge inspect surfaces v3/v4 manifest fields. Immediately after Schema ver, the human-readable header now prints:

Primary task:  converted_within_90_days
Label window:  90 days
Snapshot day:  30 days       # or "(full horizon, no windowing)" when manifest stores null
Redactions:    2 columns [current_stage, is_sql]

Rules:

  • Lines are omitted entirely for v2 bundles that don't carry these keys — no ? placeholders.
  • Redaction list: full list for ≤4 columns, first 3 + ... for ≥5 columns. The leading count is the only count printed (no redundant (N total)).
  • Pluralization is correct (1 column / N columns).
  • Snapshot day: mirrors the manifest verbatim. Only null triggers the (full horizon, no windowing) annotation; numeric values — even when equal to horizon_days — print as N days.

(B) leadforge inspect --json / -j. Dumps the parsed manifest as JSON to stdout — pipe-friendly, no header. The contract is byte-equivalent JSON to the on-disk manifest.json, asserted by test_inspect_json_equals_manifest_file.

(C) leadforge generate flags. Added --snapshot-day, --primary-task, --label-window-days (kebab-case ↔ snake_case). All Optional[int]/Optional[str] defaulting to None; recipe defaults flow through unchanged when omitted (regression test included).

(D) Help-text tightening, scoped narrowly. A pass over cli/commands/*.py revealed four help strings on generate.py flags that just restated the flag name (--n-accounts: "Number of accounts."). Those were rewritten to "Override recipe default account count."-style. Every other help string in the CLI was reviewed and left alone — they were already informative.

Deferrals closed

Item Status
M12: CLI --json flag Done (this PR)
M12: CLI help text polish Done — scope: tightened the four vague flags on generate

The deferral rationale was "no consumer needs it yet" — that stopped being true the moment v1.0 shipped to public consumers.

Explicitly out of scope

  • leadforge validate --json — separate follow-up; tracked as a new entry in .agent-plan.md's deferred table.
  • leadforge validate --strict — still needs a design call (per-check vs global gating). Remains deferred.
  • New CLI commands; CLI architecture refactor; anything outside leadforge/cli/.
  • Anything that changes manifest.json shape — this PR only consumes it.

Self-review pass (commit 68404b8)

After opening the PR, I ran a brutally honest senior-dev review of my own diff. Real problems found and fixed:

  • The snapshot_day == horizon_days → "(full horizon)" branch was silently lying about what the manifest contained. Removed. Only null triggers the annotation now.
  • v2 bundles printed Label window: ? days and similar ?-padded noise. v3+ rows are now omitted entirely when the manifest doesn't carry those keys.
  • Redaction format was self-contradictory: "6 column(s) [c1, c2, c3, ...] (6 total)" printed the count twice and used a 1990s column(s) hedge. Now: correct pluralization, no redundant (N total), line omitted entirely on empty list.
  • The truncation boundary ([:3] slice vs ≤4-cols-full rule) wasn't pinned in tests — a [:4] regression would have passed. Added explicit 4-vs-5 boundary tests that fail if c4/c5 leak into the truncated head.
  • The two _format_* helpers had runtime isinstance(cols, list) guards against malformed manifests we'd already validated. Inlined and dropped the guards.
  • Added a header-order regression test pinning the 8 pre-existing rows (the spec was explicit: don't reorder pre-existing rows; that needed an assertion).
  • Added the missing --json contract test: json.loads(stdout) == json.loads(manifest.json).
  • Added a CHANGELOG entry and a --json | jq example in the README CLI section.

Acknowledged but not unwound: this PR does four cohesive things in one diff. Splitting (B) and (C) into separate PRs would have been cleaner for blame/revert. Cost of unwinding now > cost of leaving as-is; flagging it as a process note for next time.

Test plan

  • pytest: 954 passing (937 baseline + 17 new tests after self-review)
  • ruff check . && ruff format --check .: clean
  • mypy leadforge/: clean
  • Manual smoke:
    • leadforge generate --recipe b2b_saas_procurement_v1 --seed 42 --mode student_public --out /tmp/v4smoke … produces a v4 bundle
    • leadforge inspect /tmp/v4smoke shows clean v4 fields (no (N total) redundancy, no empty-list noise)
    • leadforge inspect /tmp/v4smoke --json | jq .snapshot_day returns 30

New tests in tests/test_cli.py (17 total)

inspect:

  • surfaces v4 fields on a real bundle
  • pre-existing header rows stay in order (regression guard)
  • v2 bundle (no v3+ keys) omits the new lines entirely; no ? placeholders
  • snapshot_day=null prints the full-horizon annotation
  • snapshot_day=horizon_days prints the value verbatim, NOT relabelled
  • empty redactions: line omitted entirely
  • 1 column → singular noun (column, not column(s) or columns)
  • 2 columns: full list, plural, no ellipsis
  • 4 columns: still full list (boundary)
  • 5 columns: triggers truncation; c4/c5 must not leak into the head
  • 6 columns: same truncation shape; no (N total) suffix
  • --json output is JSON-equal to on-disk manifest.json
  • -j short flag works
  • --json and plain mode produce non-overlapping output

generate:

  • --snapshot-day flows to manifest
  • --primary-task and --label-window-days both flow to manifest
  • omitting all three new flags still yields recipe defaults (regression guard)

🤖 Generated with Claude Code

Catch the user-facing CLI up to v4 bundle schema and clear two M12
deferrals from the existing roadmap:

- `leadforge inspect` now prints `primary_task`, `label_window_days`,
  `snapshot_day` (with a "(full horizon, no windowing)" annotation
  when null/equal to horizon), and a count + list of redacted columns
  (full list for <=4 columns, truncated for more).
- `leadforge inspect --json` / `-j` dumps the parsed manifest as
  pipe-friendly JSON.
- `leadforge generate` exposes `--snapshot-day`, `--primary-task`,
  `--label-window-days` flags. They thread to existing
  `Generator.from_recipe()` kwargs; recipe defaults still apply when
  omitted.
- Help strings on `--n-accounts`, `--n-contacts`, `--n-leads`, and
  `--horizon-days` rewritten from "Number of leads." style to
  "Override recipe default ...".

Closes deferrals "M12: CLI --json flag" and "M12: CLI help text polish"
in .agent-plan.md.  `validate --json` and `--strict` remain scoped out
as separate follow-up PRs (--strict still needs a per-check vs global
gating design call).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings May 5, 2026 06:57
@shaypal5 shaypal5 added type: feature New capability layer: cli cli/ command-line interface labels May 5, 2026
@github-actions

This comment has been minimized.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates the leadforge CLI to better reflect bundle schema v4 by exposing manifest windowing/task metadata in inspect, adding a JSON output mode for scripting, and making snapshot/task config overrides available directly via generate flags.

Changes:

  • leadforge inspect now prints primary_task, label_window_days, snapshot_day (including a full-horizon annotation), and a summarized redacted_columns list.
  • Added leadforge inspect --json / -j to emit the parsed manifest as JSON to stdout.
  • Added leadforge generate flags --snapshot-day, --primary-task, --label-window-days, plus help-text improvements; tests cover new CLI behavior.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
leadforge/cli/commands/inspect.py Adds --json/-j mode and surfaces v4 manifest fields (task/windowing/redactions) in human-readable output.
leadforge/cli/commands/generate.py Exposes task/windowing overrides as first-class CLI flags and improves help text for population/horizon overrides.
tests/test_cli.py Adds integration tests for new generate flags, inspect v4 field surfacing, redaction formatting, and JSON output behavior.
.agent-plan.md Updates milestone deferrals table to reflect completed CLI polish items and newly deferred validate --json.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread leadforge/cli/commands/inspect.py Outdated
Acts on the brutal self-review of #60.  Summary:

- Drop the snapshot_day == horizon_days "(full horizon)" branch.  The
  manifest is the source of truth; if a user pinned snapshot_day=90 on
  a 90-day horizon, inspect now prints "90 days" verbatim instead of
  silently relabelling it.  Only manifest snapshot_day=null prints the
  full-horizon annotation.
- Skip v3+ rows (Primary task / Label window / Snapshot day /
  Redactions) entirely when the manifest doesn't carry those keys —
  v2 bundles render cleanly with no "?" placeholders.
- Inline the two _format_* helpers; drop the redundant runtime
  isinstance() guards (manifest dict-shape was already validated up
  top).
- Redaction line cleanup: pluralize correctly ("1 column" /
  "N columns"), drop the "(N total)" redundancy, omit the line
  entirely on empty redacted_columns instead of printing "0 column(s) []".
- Pin the truncation boundary explicitly in tests: 4 cols → full
  list, 5 cols → first-3 + ellipsis (assertions fail if c4/c5 leak
  into the truncated head).
- Add header-order regression test pinning the 8 pre-existing rows.
- Add the missing --json contract test: stdout is JSON-equivalent to
  on-disk manifest.json.
- Add CHANGELOG entry under Unreleased and a `--json | jq` example
  to the README CLI section.

All 954 tests pass; ruff + mypy clean.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@github-actions

github-actions Bot commented May 5, 2026

Copy link
Copy Markdown

pr-agent-context report:

This run includes an unresolved review comment on PR #60 in repository https://github.com/leadforge-dev/leadforge

For each unresolved review comment, recommend one of: resolve as irrelevant, accept and implement
the recommended solution, open a separate issue and resolve as out-of-scope for this PR, accept and
implement a different solution, or resolve as already treated by the code.

After I reply with my decision per item, implement the accepted actions, resolve the corresponding
PR comments, and push all of these changes in a single commit.

# Copilot Comments

## COPILOT-1
Location: leadforge/cli/commands/inspect.py
URL: https://github.com/leadforge-dev/leadforge/pull/60#discussion_r3186586897
Status: outdated
Root author: copilot-pull-request-reviewer

Comment:
    `Label window` always appends the literal " days" even when `label_window_days` is missing or not an int, producing output like "? days" for older bundles / minimal manifests. Consider formatting this field like `_format_snapshot_day()` (only add the units when the value is an int, otherwise print just "?") to avoid confusing human-readable output.

Run metadata:

Tool ref: v4
Tool version: 4.0.21
Trigger: commit pushed
Workflow run: 25362619927 attempt 1
Comment timestamp: 2026-05-05T07:04:55.403312+00:00
PR head commit: 68404b87063ca9073ab6eb533d35b31d411f2b78

@shaypal5 shaypal5 merged commit 8dc21c1 into main May 5, 2026
9 of 10 checks passed
@shaypal5 shaypal5 deleted the m12-cli-polish branch May 5, 2026 07:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

layer: cli cli/ command-line interface type: feature New capability

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants