feat: CLI v2 — nested commands, queries, and rich output#65
Conversation
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace flat command registration with nested subcommand groups (bootstrap, sync, update, fetch, query, db). Reuse all existing task logic — just reorganize the Commander tree. Old command files are preserved but no longer imported. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add queryEntities function with support for CIK lookup, SIC/state filtering, partial name search, sorting, and pagination. Wire into the query entities CLI subcommand with table/json/csv output. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add queryFilings() with support for CIK, form type, date range, and text search filters. Wire into the query filings subcommand. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…facts, persons) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
All 14 old command files replaced by nested group structure in src/cli/groups/. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Verifies the command hierarchy works end-to-end by spawning the CLI as a subprocess and checking help output for all command groups, global options, version, and subcommands (bootstrap, query, fetch, update, db). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Redesigns the sec CLI into a v2 command hierarchy with shared global flags, richer output helpers, and a new query surface over stored SEC data—backed by updated specs/docs and a CLI smoke test.
Changes:
- Replaces the legacy flat command set with nested command groups (
bootstrap,sync,update,fetch,query,db,init). - Adds shared CLI infrastructure (global options parsing,
runCommandwrapper, table/progress utilities) and new query modules + tests. - Updates documentation/specs and adds an integration test for the CLI help hierarchy.
Reviewed changes
Copilot reviewed 52 out of 53 changed files in this pull request and generated 16 comments.
Show a summary per file
| File | Description |
|---|---|
| src/sec.ts | CLI entrypoint updated to v2 + global options hookup |
| src/commands/index.ts | Switches command registration to new grouped CLI modules |
| src/commands/UpdateAllSubmissions.ts | Removes legacy flat command |
| src/commands/UpdateAllForms.ts | Removes legacy flat command |
| src/commands/UpdateAllCompanyFacts.ts | Removes legacy flat command |
| src/commands/Submissions.ts | Removes legacy flat command |
| src/commands/SetupDB.ts | Removes legacy flat command |
| src/commands/Form.ts | Removes legacy flat command |
| src/commands/Doc.ts | Removes legacy flat command |
| src/commands/DailyIndex.ts | Removes legacy flat command |
| src/commands/CompanyFacts.ts | Removes legacy flat command |
| src/commands/BootstrapSubmissions.ts | Removes legacy flat command |
| src/commands/BootstrapDownload.ts | Removes legacy flat command |
| src/commands/BootstrapCompanyFacts.ts | Removes legacy flat command |
| src/commands/BootstrapCikLastUpdate.ts | Removes legacy flat command |
| src/commands/BootstrapAllCikNames.ts | Removes legacy flat command |
| src/cli/runCommand.ts | Adds centralized command wrapper for exit codes + stderr errors |
| src/cli/runCommand.test.ts | Tests for runCommand behavior |
| src/cli/queries/PersonQuery.ts | Adds “persons” query module |
| src/cli/queries/PersonQuery.test.ts | Tests for persons query |
| src/cli/queries/OfferingQuery.ts | Adds “offerings” query module |
| src/cli/queries/OfferingQuery.test.ts | Tests for offerings query |
| src/cli/queries/FilingQuery.ts | Adds “filings” query module |
| src/cli/queries/FilingQuery.test.ts | Tests for filings query |
| src/cli/queries/FactsQuery.ts | Adds “facts” query module |
| src/cli/queries/FactsQuery.test.ts | Tests for facts query |
| src/cli/queries/EntityQuery.ts | Adds “entities” query module (with sorting support) |
| src/cli/queries/EntityQuery.test.ts | Tests for entities query |
| src/cli/queries/DbStatus.ts | Adds DB inspection helpers (status/stats) |
| src/cli/queries/DbStatus.test.ts | Tests for DB inspection helpers |
| src/cli/queries/CrowdfundingQuery.ts | Adds “crowdfunding” query module |
| src/cli/queries/CrowdfundingQuery.test.ts | Tests for crowdfunding query |
| src/cli/output/index.ts | Barrel exports for output helpers |
| src/cli/output/TableRenderer.ts | Adds table/csv/json renderer utility |
| src/cli/output/TableRenderer.test.ts | Tests for table renderer |
| src/cli/output/Progress.ts | Adds status/progress/spinner helper primitives |
| src/cli/output/Progress.test.ts | Tests for progress/spinner helpers |
| src/cli/groups/update.ts | Adds sec update … command group |
| src/cli/groups/sync.ts | Adds sec sync pipeline command |
| src/cli/groups/query.ts | Adds sec query … command group wired to query modules |
| src/cli/groups/init.ts | Adds interactive sec init wizard + .env.local generation |
| src/cli/groups/init.test.ts | Tests for .env.local generation logic |
| src/cli/groups/fetch.ts | Adds sec fetch … command group |
| src/cli/groups/db.ts | Adds sec db … command group (setup/status/stats/reset) |
| src/cli/groups/bootstrap.ts | Adds sec bootstrap … command group + pipeline |
| src/cli/cli.integration.test.ts | CLI smoke/integration test verifying command tree + global flags |
| src/cli/GlobalOptions.ts | Adds global CLI flags + parsing |
| src/cli/GlobalOptions.test.ts | Tests for global flags |
| package.json | Dependency adjustments (incl. bunset bump) |
| bun.lock | Lockfile updates for dependency changes |
| docs/plans/2026-03-05-cli-v2-plan.md | Implementation plan doc added |
| docs/plans/2026-03-05-cli-v2-design.md | CLI v2 design doc added |
| SPEC.md | CLI spec updated to v2 structure |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } finally { | ||
| rl.close(); |
There was a problem hiding this comment.
The readline interface is closed twice (rl.close() inside the try block and again in finally). This can lead to inconsistent behavior across Node/Bun versions. Prefer closing exactly once (e.g., remove the inner rl.close() and rely on finally, or guard with a boolean).
| } finally { | |
| rl.close(); |
There was a problem hiding this comment.
use the finally version
| const limit = parseInt(options.limit); | ||
| const offset = parseInt(options.offset); |
There was a problem hiding this comment.
limit/offset are parsed with parseInt without validation. If the user passes a non-numeric value, NaN will flow into .slice() and silently return empty results. Use a commander option parser (like the existing parseIntOption) or explicitly validate and throw a user-friendly error when parsing fails.
| const limit = parseInt(options.limit); | |
| const offset = parseInt(options.offset); | |
| const limit = Number.parseInt(options.limit, 10); | |
| if (Number.isNaN(limit)) { | |
| throw new Error(`Invalid value for --limit: "${options.limit}" is not a valid integer.`); | |
| } | |
| const offset = Number.parseInt(options.offset, 10); | |
| if (Number.isNaN(offset)) { | |
| throw new Error(`Invalid value for --offset: "${options.offset}" is not a valid integer.`); | |
| } |
| console.log( | ||
| renderTable(result.rows as Record<string, unknown>[], columns, { | ||
| format: options.format as "table" | "csv" | "json", | ||
| total: result.total, | ||
| offset, | ||
| limit, | ||
| }) | ||
| ); |
There was a problem hiding this comment.
options.format is an unvalidated string from commander but is cast to "table" | "csv" | "json" and passed through. An invalid value will currently surface as undefined output (via renderTable). Validate --format against allowed values (or set commander .choices([...])) and fail fast with a clear error message.
| | `--from <date>` | Filing date start (YYYY-MM-DD) | | ||
| | `--to <date>` | Filing date end (YYYY-MM-DD) | |
There was a problem hiding this comment.
SPEC documents sec query filings date flags as --from/--to, but the implementation uses --after/--before. Update SPEC.md to match the implemented flags (or add aliases in the CLI for compatibility).
| | `--from <date>` | Filing date start (YYYY-MM-DD) | | |
| | `--to <date>` | Filing date end (YYYY-MM-DD) | | |
| | `--after <date>` | Filing date start (YYYY-MM-DD) | | |
| | `--before <date>` | Filing date end (YYYY-MM-DD) | |
| const start = offset + 1; | ||
| const end = offset + count; | ||
| lines.push(""); | ||
| lines.push(`Showing ${start}-${end} of ${options.total} results`); | ||
|
|
||
| if (end < options.total) { |
There was a problem hiding this comment.
renderTextTable can produce an invalid pagination range when rows.length === 0 but options.total is provided (e.g., "Showing 1-0 of 10 results"). Adjust the footer logic to handle empty pages (e.g., show "Showing 0-0" or omit the range when count is 0).
| const start = offset + 1; | |
| const end = offset + count; | |
| lines.push(""); | |
| lines.push(`Showing ${start}-${end} of ${options.total} results`); | |
| if (end < options.total) { | |
| const start = count === 0 ? 0 : offset + 1; | |
| const end = count === 0 ? 0 : offset + count; | |
| lines.push(""); | |
| lines.push(`Showing ${start}-${end} of ${options.total} results`); | |
| if (count > 0 && end < options.total) { |
|
|
||
| Detects existing `.env.local` and offers to reconfigure or skip. PostgreSQL path prompts for connection string or individual parameters. Validates database connectivity before writing config. | ||
|
|
||
| When `--json` is passed: outputs config as JSON, no interactivity (for scripting). Non-zero exit if any step fails. |
There was a problem hiding this comment.
This design doc claims sec init --json outputs config as JSON with no interactivity, but the current init implementation is always interactive and does not handle --json. Either implement the documented behavior or adjust the design doc to reflect the current CLI capabilities.
| When `--json` is passed: outputs config as JSON, no interactivity (for scripting). Non-zero exit if any step fails. | |
| `sec init` is always interactive in v2. Non-zero exit if any step fails. |
| | `--verbose` | `-v` | Enable detailed log output | | ||
| | `--dry-run` | | Show what would be done without making changes | | ||
| | `--no-color` | | Disable colored output | | ||
| | `--concurrency` | `-c` | Max parallel operations (default varies by command)| |
There was a problem hiding this comment.
SPEC lists short flags -v for --verbose and -c for --concurrency, but applyGlobalOptions currently only defines the long forms. Either add the short aliases in code or remove the short forms from the spec to avoid confusing users.
| | `--verbose` | `-v` | Enable detailed log output | | |
| | `--dry-run` | | Show what would be done without making changes | | |
| | `--no-color` | | Disable colored output | | |
| | `--concurrency` | `-c` | Max parallel operations (default varies by command)| | |
| | `--verbose` | | Enable detailed log output | | |
| | `--dry-run` | | Show what would be done without making changes | | |
| | `--no-color` | | Disable colored output | | |
| | `--concurrency` | | Max parallel operations (default varies by command)| |
| export function buildEnvConfig(config: InitConfig): string { | ||
| const lines: string[] = [ | ||
| `SEC_DB_TYPE="${config.dbType}"`, | ||
| `SEC_DB_FOLDER="${config.dbFolder}"`, | ||
| `SEC_DB_NAME="${config.dbName}"`, | ||
| `SEC_RAW_DATA_FOLDER="${config.rawDataFolder}"`, | ||
| ]; | ||
|
|
||
| if (config.dbType === "postgres") { | ||
| if (config.pgUrl) { | ||
| lines.push(`SEC_PG_URL="${config.pgUrl}"`); | ||
| } else { | ||
| if (config.pgHost) lines.push(`SEC_PG_HOST="${config.pgHost}"`); | ||
| if (config.pgPort) lines.push(`SEC_PG_PORT="${config.pgPort}"`); | ||
| if (config.pgUser) lines.push(`SEC_PG_USER="${config.pgUser}"`); | ||
| if (config.pgPassword) lines.push(`SEC_PG_PASSWORD="${config.pgPassword}"`); | ||
| if (config.pgDatabase) lines.push(`SEC_PG_DATABASE="${config.pgDatabase}"`); | ||
| } | ||
| } | ||
|
|
||
| return lines.join("\n") + "\n"; |
There was a problem hiding this comment.
buildEnvConfig interpolates user-provided values into double-quoted .env lines without escaping. If a value contains " or newlines (e.g., a PostgreSQL password), it can break the file format or inject additional variables. Escape " and \n (or switch to a safe .env encoder) before writing.
| .command("submissions") | ||
| .description("Update all submissions for all companies") | ||
| .option("--concurrency <n>", "Override default concurrency") | ||
| .action(async () => { | ||
| await runCommand(async () => { | ||
| await runTasks(new UpdateAllSubmissionsTask()); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
These subcommands define a --concurrency option but the handler ignores it, and the underlying tasks currently hardcode their concurrency limits. This makes the CLI misleading. Either remove the option or plumb the value through (e.g., extend task input to accept a concurrency limit and use it in wf.map({ concurrencyLimit })).
| #### `sec query entities [cik]` | ||
|
|
||
| List or look up entities. | ||
|
|
||
| | Argument | Required | Description | | ||
| | -------- | -------- | --------------------------------- | | ||
| | `cik` | No | Specific CIK to look up | | ||
|
|
||
| | Option | Description | | ||
| | ------------------ | ----------------------------- | | ||
| | `--name <pattern>` | Filter by name (LIKE pattern) | | ||
| | `--sic <code>` | Filter by SIC code | | ||
| | `--state <code>` | Filter by state of incorporation | | ||
| | `--limit <n>` | Max rows (default: 25) | | ||
| | `--offset <n>` | Skip rows (default: 0) | | ||
| | `--format <fmt>` | Output format: table, csv, json | |
There was a problem hiding this comment.
SPEC documents sec query entities [cik] with a positional CIK argument and --name, but the implemented CLI is sec query entities [search] with --cik (and no --name). Please align the spec to the actual command signature/flags (or update the command to match the spec).
| #### `sec query entities [cik]` | |
| List or look up entities. | |
| | Argument | Required | Description | | |
| | -------- | -------- | --------------------------------- | | |
| | `cik` | No | Specific CIK to look up | | |
| | Option | Description | | |
| | ------------------ | ----------------------------- | | |
| | `--name <pattern>` | Filter by name (LIKE pattern) | | |
| | `--sic <code>` | Filter by SIC code | | |
| | `--state <code>` | Filter by state of incorporation | | |
| | `--limit <n>` | Max rows (default: 25) | | |
| | `--offset <n>` | Skip rows (default: 0) | | |
| | `--format <fmt>` | Output format: table, csv, json | | |
| #### `sec query entities [search]` | |
| List or look up entities. | |
| | Argument | Required | Description | | |
| | -------- | -------- | --------------------------------------- | | |
| | `search` | No | Free-text search term to filter entities | | |
| | Option | Description | | |
| | --------------- | ------------------------------------ | | |
| | `--cik <cik>` | Filter by exact CIK | | |
| | `--sic <code>` | Filter by SIC code | | |
| | `--state <code>`| Filter by state of incorporation | | |
| | `--limit <n>` | Max rows (default: 25) | | |
| | `--offset <n>` | Skip rows (default: 0) | | |
| | `--format <fmt>`| Output format: table, csv, json | |
|
@copilot open a new pull request to apply changes based on the comments in this thread |
…, and docs alignment (#66) * Initial plan * fix: apply review feedback — validation, escaping, type correctness, and docs alignment Co-authored-by: sroussey <127349+sroussey@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: sroussey <127349+sroussey@users.noreply.github.com>
|
@copilot why is the test ending in an error when it has no failures? |
Updated the test suite for runCommand to ensure process.exitCode is reset to 0 after each test and after all tests have completed, improving test reliability.
…and handling Updated the command registration process to initialize dependency injection only once, triggered by a preAction hook. This change enhances performance and ensures that the DI setup is not repeated unnecessarily for each command execution.
Summary
Complete redesign of the CLI for better ergonomics:
sec init→sec bootstrap→sec synccovers 90% of use casesbootstrap,sync,update,fetch,query,dbreplace 14 flat commandssec query entities|filings|offerings|crowdfunding|facts|personswith--format table|csv|json--json,--verbose,--dry-run,--no-color,--concurrencyrunCommandwrapper.env.localgenerationsec db statusandsec db statsBefore → After
setup-dbsec db setup(orsec init)bootstrap-download submissionssec bootstrap download submissionsbootstrap-all-cik-namessec bootstrap ingest cik-namesbootstrap-submissionssec bootstrap ingest submissionssubmissions 1018724sec fetch submissions 1018724update-all-submissionssec update submissionssec query entities Teslasec syncStats
Test plan
bun test)bun run build)resetDependencyInjectionsForTesting()🤖 Generated with Claude Code