feat: add --verbose flag to commit-echo suggest#74
Conversation
Adds -v/--verbose flag to 'commit-echo suggest' that prints: - Model name and provider - Diff size (or truncation details) - Style profile summary (avg length, imperative mood rate) - Number of commits analyzed
404-Page-Found
left a comment
There was a problem hiding this comment.
PR Review: feat: add --verbose flag to commit-echo suggest (#74)
Verdict: Request Changes 🟡
The PR adds a useful -v/--verbose flag and is generally well-structured. However, there are a few issues that should be addressed before merging.
What the PR does well
- Focused scope — Single feature, clean 20-line delta.
- Good naming —
-v/--verbosefollows CLI conventions. - Consistent styling — Uses
pc.dim()for verbose output, same pattern as existing profile display. - Edge cases handled:
- Profile stats only shown when
profile.totalCommits > 0(no crash on empty history) - Diff size only shown when no truncation occurred (
else ifensures mutual exclusion) - Flag is properly converted to boolean with
Boolean(options.verbose)preventing Commander's default string behavior
- Profile stats only shown when
- No breaking changes — Existing behavior untouched when
--verboseis omitted.
Issues to address
1. Missing automated tests (Important)
The PR adds a new user-facing CLI flag but includes no tests. The project has an existing test suite with patterns like tests/e2e/suggest-smoke.test.mjs that mock a server and test the CLI end-to-end. A similar test should verify that:
commit-echo suggest -vexits successfully- Verbose output (model, provider, diff size, or profile stats) appears on stdout when the flag is set
- Without
--verbose, output is unchanged (no regression)
2. Unnecessary undefined guard on imperativeRate (Nitpick → should clean up)
if (profile.imperativeRate !== undefined) {StyleProfile.imperativeRate is typed as number (not optional), and the outer check profile.totalCommits > 0 guarantees buildProfile returns a meaningful value. This guard is dead code and should be removed.
3. Inconsistent abbreviation with existing formatProfile (Nitpick)
The existing formatProfile() displays:
Average length: 42 characters
But the new verbose output uses:
Avg length: 42 chars
Consider aligning the wording to match the existing convention ("characters" instead of "chars").
Security ✅
No new dependencies, no exposed secrets, no injection vectors.
Performance ✅
Negligible — adds a handful of console.log calls.
Documentation ✅
-v, --verbose is properly registered in Commander. The PR description documents what each verbose line produces.
Summary: The feature is well-conceived and the implementation is mostly clean. Please add tests for the new flag, remove the dead-code guard, and consider the wording nitpick above.
|
Hi @lb1192176991-lab, just checking in on this PR. Are you still planning to work on it, or has it been abandoned? |
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| ErrorProne | 1 high |
🟢 Metrics 13 complexity · 0 duplication
Metric Results Complexity 13 Duplication 0
AI Reviewer: first review requested successfully. AI can make mistakes. Always validate suggestions.
TIP This summary will be updated as you push new changes.
There was a problem hiding this comment.
Pull Request Overview
The current implementation introduces a --verbose flag but results in an inconsistent CLI experience and lacks automated verification. The PR is currently marked as not up to standards by Codacy due to high-severity quality findings and a significant increase in complexity in src/commands/suggest.ts (+9).
Specifically, the root command commit-echo invokes the suggestion logic but does not support the new flag, which will cause 'unknown option' errors for users. Additionally, several technical debt items such as redundant async operations and unsafe type assertions must be addressed to satisfy quality standards.
About this PR
- The
--verboseflag is inconsistent across the CLI. Since the rootcommit-echocommand can invoke the suggestion logic by default, it must also support and pass this flag to prevent 'unknown option' errors. - No new test scenarios were included in this PR to verify the correct rendering of diagnostic output or the parsing of the new flag.
1 comment outside of the diff
src/commands/suggest.ts
line 20🔴 HIGH RISK
Suggestion: The displaySuggestions function does not perform any asynchronous operations. Removing the async keyword improves performance and resolves the linting errors regarding unhandled async errors and unnecessary async declarations.
Test suggestions
- Missing recommended test scenario: Verify that 'suggest --verbose' outputs the correct model and provider information from the config.
- Missing recommended test scenario: Verify that 'suggest -v' outputs the style profile statistics when at least one commit exists in history.
- Missing recommended test scenario: Verify that 'suggest --verbose' outputs the total diff size when the diff is within limits (no truncation).
- Missing recommended test scenario: Verify that running 'suggest' without the verbose flag does not print any of the new diagnostic information.
Prompt proposal for missing tests
Consider implementing these tests if applicable:
1. Missing recommended test scenario: Verify that 'suggest --verbose' outputs the correct model and provider information from the config.
2. Missing recommended test scenario: Verify that 'suggest -v' outputs the style profile statistics when at least one commit exists in history.
3. Missing recommended test scenario: Verify that 'suggest --verbose' outputs the total diff size when the diff is within limits (no truncation).
4. Missing recommended test scenario: Verify that running 'suggest' without the verbose flag does not print any of the new diagnostic information.
TIP Improve review quality by adding custom instructions
TIP How was this review? Give us feedback
| .option('--commit', 'Commit the selected suggestion', false) | ||
| .option('-y, --yes', 'Automatically select the first suggestion and skip prompts') | ||
| .option('--auto', 'Alias for --yes') | ||
| .option('-v, --verbose', 'Show verbose diagnostic info') |
There was a problem hiding this comment.
🟡 MEDIUM RISK
Register the -v, --verbose option on the root program so it is available globally, and ensure the root action passes the verbose option to the underlying suggest command.
What
Added
-v/--verboseflag to thecommit-echo suggestcommand that prints extra diagnostic information.Why
Makes it easier to debug and understand what the tool is doing under the hood — which model/provider is being used, whether the diff was truncated, and the style profile statistics.
Verbose output includes
pc.dim()for less prominent stylingTesting
commit-echo suggest -vworks as alias for--verbose--verbose, output is unchanged (no breaking change)npm run buildpasses