-
Notifications
You must be signed in to change notification settings - Fork 0
feat(json-reporter): make compact output the default #197
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
dd7cd96 to
14d9a7a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR changes the JSON reporter's default output format from pretty-printed to compact JSON, introducing a new --json-pretty CLI flag and reporterConfig.json.prettyPrint configuration option to opt into formatted output. The change improves the type safety of reporter configuration by deriving types from Zod schemas using z.infer.
Key changes:
- JSON reporter now outputs compact (single-line) JSON by default instead of pretty-printed
- Added
--json-prettyCLI flag with proper precedence (CLI > config file > default) - Refactored type system to derive
ModestBenchConfig,ReporterConfig, andJsonReporterConfigfrom Zod schemas
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/integration/json-pretty.test.ts | New integration tests validating default compact output, CLI flag, config file option, and precedence rules |
| src/types/core.ts | Added type imports from schema, exported schema-derived types for ReporterConfig and JsonReporterConfig, reorganized exports |
| src/services/config-manager.ts | Added TODO comment about budget transformation type mismatch |
| src/reporters/json.ts | Changed prettyPrint default from true to false |
| src/config/schema.ts | Added jsonReporterConfigSchema and reporterConfigSchema, moved ModestBenchConfig type definition here as schema-derived type |
| src/cli/index.ts | Added --json-pretty flag option, changed default JsonReporter registration to prettyPrint: false |
| src/cli/commands/run.ts | Updated RunOptions interface, implemented precedence logic for prettyPrint (CLI > config > default) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Change JSON reporter default from prettyPrint: true to false - Add --json-pretty CLI flag to opt-in to formatted output - Support reporterConfig.json.prettyPrint in config files - CLI flag takes precedence over config file setting - Derive ReporterConfig and JsonReporterConfig from Zod schemas - Add TODO for ThresholdConfig schema derivation - Add integration tests for CLI flag and config precedence BREAKING CHANGE: JSON reporter now outputs compact JSON by default. Use --json-pretty flag or set reporterConfig.json.prettyPrint: true in config file to get formatted output. Closes #74 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
14d9a7a to
239b83d
Compare
Address PR review comments: - Move createRunId/createTaskId re-export near other type re-exports to avoid separating JSDoc from its ReporterConfig type definition - Add test for --no-json-pretty overriding config prettyPrint: true to ensure comprehensive CLI precedence coverage 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Address PR review comments: - Remove TODO referencing unrelated issue #15 (it was noise added by accident - ThresholdConfig can be derived from schema in a separate PR) - Expand comment explaining why empty config is used in default behavior test to clarify config isolation strategy 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
BREAKING CHANGE: JSON reporter now outputs compact JSON by default.
Use --json-pretty flag or set reporterConfig.json.prettyPrint: true
in config file to get formatted output.
Closes #74
🤖 Generated with Claude Code
Co-Authored-By: Claude [email protected]