feat(cli): add color output for scores, dimensions, and issues#37
feat(cli): add color output for scores, dimensions, and issues#37riddhij-7 wants to merge 1 commit into
Conversation
|
Thanks for the color output! This is a useful QoL addition but it's a follow-up to the MVP rather than part of the foundational work. I'm leaving this PR open rather than merging it on the integration branch. Two options for moving forward:
No code changes requested — your PR is in good shape. Just wanted to confirm it's queued for after MVP, not lost. Appreciate the contribution! |
|
Hi @riddhij-7 — main has been integrated via PR #85 (merged at 251140c) and is stable. The color output feature is still useful and the screenshots in the PR description look great. The PR now needs a rebase against current
These changes are why a test merge of your branch into To bring this home:
No code review feedback from me yet on the feature itself — it looks good. Happy to merge once the rebase lands. If you'd rather I handle the lockfile regeneration on the maintainer side to keep your diff minimal, just say so. |
docs/MVP_DEMO_PLAN.md still said 'Node 22+' in the prereqs section, which conflicts with the actual repo metadata (Node >= 20.0.0 in package.json) and with the authoritative setup guide in docs/LOCAL_DEMO.md. Replace the prereqs with a short pointer to LOCAL_DEMO.md plus a one-line accurate summary, so this historical demo-readiness doc no longer contradicts current setup instructions. docs/REPO_DOCS_AUDIT.md: remove the corresponding entry from the 'remaining documentation risks' list, fix the resulting item numbering, and add a note recording that the Node-version item was resolved before merge. No code, evaluator, or rule changes. No new features. PR #37 and PR #78 untouched. Validates: pnpm test 12/12, pnpm lint 0 errors, pnpm build 6/6 (fresh --force, emits /, /_not-found, /feedback, /results).
Two unresolved CodeRabbit comments fixed in this commit: 1. README.md — replace hard-coded '5 issues' in the evaluator diagram with the non-fixed wording 'Issues'. evaluate() returns a variable-length issues array; a fixed count would drift as scoring changes. The other diagram labels (Score, Strengths, ATS verdict, Archetype) are kept as-is because they describe deterministic outputs that do not vary in count. 2. packages/intelligence/README.md — the previous audit pass claimed this package ships eight roles and falls back to Backend Engineer. But packages/intelligence/src/archetypes/index.ts registers only three archetypes (Software Engineer, Product Manager, Data & ML Engineer) and DEFAULT_ARCHETYPE is softwareEngineer. Update the README to reflect the actual registry. Add a note clarifying that @cv-builder/core has a separate, broader legacy/runtime registry (7 roles) used by the CLI and Web UI, and that unifying the two registries is a follow-up — see docs/ARCHETYPE_GAP_AUDIT.md. docs/REPO_DOCS_AUDIT.md updated to record that the packages/intelligence/README.md archetype-inventory row was corrected in two steps (the audit pass incorrectly bumped the package claim to 8; this commit brings it back to 3 and adds the @cv-builder/core note). No code changes. No new features. PR #37 and PR #78 untouched. Docs only. Validates: pnpm test 12/12, pnpm lint 0 errors, pnpm build 6/6.
* docs: align repository documentation with MVP status Documentation-only audit. No code, evaluator, or rule changes; no new features; no Cloudflare work; no changes to PR #37 or #78. User-facing copy fixed: - README.md: replace misleading ASCII diagram (3 rewrites / Tailored CV) with the actual MVP outputs (Score, Issues, Strengths, ATS verdict, Archetype); clarify that /evaluate-cv ./my-resume.pdf works only because Claude Code reads PDFs natively (the local CLI/web UI parse .md and .txt only). - apps/web-ui layout.tsx: Next.js metadata description replaced with honest CV-evaluator copy (no longer 'Build a tailored resume...'). - apps/web-ui/README.md: list all three routes (/, /results, /feedback), note the static-export + privacy-first posture. - apps/cli/README.md: same PDF-clarification note as the root README. - package.json descriptions (root + core + cli): remove 'tailor / build' wording; describe the deterministic evaluator. - packages/intelligence/README.md: list the eight shipped roles (matches README and the actual implementation) and correct the default archetype (Backend Engineer, not Software Engineer). - packages/eval/README.md: drop the outdated LLM-provider claim; the MVP is fully deterministic. - .claude/skills/cv-evaluation/SKILL.md: 'local MVP', not 'hosted product'; default archetype corrected. - ROADMAP.md: status note added; Phase-1 / #74 / #75 / #76 / #85 / #87 marked as recently shipped; only #37 and #78 remain in progress. Historical docs marked with status notes (not rewritten, just flagged as pre-release context): ARCHITECTURE.md, PROPOSAL.md, PHASE-1.md, V1_SCOPE.md, MVP_DEMO_PLAN.md, PR_CLEANUP_HANDOFF.md, POST_MERGE_VALIDATION.md, REMAINING_PRS_PLAN.md, PR85_ROLLBACK_PLAN.md. Each now points readers to docs/MVP_RELEASE_STATUS.md. New: - docs/REPO_DOCS_AUDIT.md records the audit date, files reviewed, files changed, outdated claims found and fixed, historical docs left intentionally unchanged, remaining risks, and the validation results. Validates: pnpm test (12/12), pnpm lint (0 errors), pnpm build (6/6, fresh --force: emits /, /_not-found, /feedback, /results). * docs: fix remaining Node version reference docs/MVP_DEMO_PLAN.md still said 'Node 22+' in the prereqs section, which conflicts with the actual repo metadata (Node >= 20.0.0 in package.json) and with the authoritative setup guide in docs/LOCAL_DEMO.md. Replace the prereqs with a short pointer to LOCAL_DEMO.md plus a one-line accurate summary, so this historical demo-readiness doc no longer contradicts current setup instructions. docs/REPO_DOCS_AUDIT.md: remove the corresponding entry from the 'remaining documentation risks' list, fix the resulting item numbering, and add a note recording that the Node-version item was resolved before merge. No code, evaluator, or rule changes. No new features. PR #37 and PR #78 untouched. Validates: pnpm test 12/12, pnpm lint 0 errors, pnpm build 6/6 (fresh --force, emits /, /_not-found, /feedback, /results). * docs: address remaining CodeRabbit comments on PR #88 Two unresolved CodeRabbit comments fixed in this commit: 1. README.md — replace hard-coded '5 issues' in the evaluator diagram with the non-fixed wording 'Issues'. evaluate() returns a variable-length issues array; a fixed count would drift as scoring changes. The other diagram labels (Score, Strengths, ATS verdict, Archetype) are kept as-is because they describe deterministic outputs that do not vary in count. 2. packages/intelligence/README.md — the previous audit pass claimed this package ships eight roles and falls back to Backend Engineer. But packages/intelligence/src/archetypes/index.ts registers only three archetypes (Software Engineer, Product Manager, Data & ML Engineer) and DEFAULT_ARCHETYPE is softwareEngineer. Update the README to reflect the actual registry. Add a note clarifying that @cv-builder/core has a separate, broader legacy/runtime registry (7 roles) used by the CLI and Web UI, and that unifying the two registries is a follow-up — see docs/ARCHETYPE_GAP_AUDIT.md. docs/REPO_DOCS_AUDIT.md updated to record that the packages/intelligence/README.md archetype-inventory row was corrected in two steps (the audit pass incorrectly bumped the package claim to 8; this commit brings it back to 3 and adds the @cv-builder/core note). No code changes. No new features. PR #37 and PR #78 untouched. Docs only. Validates: pnpm test 12/12, pnpm lint 0 errors, pnpm build 6/6. --------- Co-authored-by: Cleanup Bot <cleanup-bot@example.com>
The audit draft was written before the GitHub API assigned the actual issue numbers. It referenced 'issue #97' in three places for the plain-Product-Manager follow-up. The real issue number is #90 (verified via gh issue view). - Line 135: 'issue #97, see footer' -> 'issue #90, see footer' - Line 194: 'Filed as issue #97' -> 'Filed as issue #90' - Line 204: 'follow-up to #97' -> 'follow-up to #90' Also caught by this rebase onto latest main (docs: align repository documentation with MVP status, PR #88). The rebase was conflict-free because PR #88 and the audit modified orthogonal sections of the shared files (README.md, docs/MVP_RELEASE_STATUS.md, and docs/PR_CLEANUP_HANDOFF.md). The rebase confirmed that all five audit findings survived intact: - core registry has 7 live archetypes - intelligence registry has 3 archetypes - plain Product Manager is missing from core - #90 tracks plain Product Manager - #91 tracks registry consolidation No code changes. Docs only. PR #37 and PR #78 untouched. Validates: pnpm test 12/12, pnpm lint 0 errors, pnpm build 6/6.
* docs: audit role archetype coverage
Adds a comprehensive audit of the role-archetype registries, plus the
small docs-only corrections called out in the audit.
Audited:
- packages/core/src/archetypes/index.ts (7 archetypes, RoleArchetype)
- packages/intelligence/src/archetypes/ (3 archetypes, Archetype)
- packages/schemas/src/archetype.ts (Zod Archetype schema)
- packages/core/src/evaluator/index.ts (live evaluator)
- packages/cli/src/cli.ts (CLI archetypes list)
- apps/web-ui/src/app/{results,components}/* (web UI consumer)
- packages/eval/src/__tests__/fixtures.test.ts (eval fixtures)
- issues, scripts/create-issues.sh, docs/ISSUES_SEED.md, ROADMAP
equivalents in README/docs, V1_SCOPE, PHASE-1, ARCHITECTURE
Key findings:
1. Two parallel archetype registries (core vs intelligence) on
divergent schemas. CLI + Web UI use core (7); prompts + eval
fixtures use intelligence (3).
2. README claimed 8 archetypes; core has 7, intelligence has 3.
Fixed to 7 in README, MVP_RELEASE_STATUS, PR_CLEANUP_HANDOFF.
3. Plain Product Manager detection is broken in the live system:
core has only ai-product-manager, so a non-AI PM CV falls back
to backend-engineer.
Files changed:
- docs/ARCHETYPE_GAP_AUDIT.md (new, 200+ lines)
- README.md: '8 role archetypes' -> '7', removed Machine Learning
Engineer from 'currently built-in' list (not in core), added
pointer to the audit
- docs/MVP_RELEASE_STATUS.md: 8 archetypes -> 7 in two places
- docs/PR_CLEANUP_HANDOFF.md: clarified 3 archetypes in intelligence
vs 7 in core
* docs: fix stale issue numbers in archetype gap audit
The audit draft was written before the GitHub API assigned the actual
issue numbers. It referenced 'issue #97' in three places for the
plain-Product-Manager follow-up. The real issue number is #90
(verified via gh issue view).
- Line 135: 'issue #97, see footer' -> 'issue #90, see footer'
- Line 194: 'Filed as issue #97' -> 'Filed as issue #90'
- Line 204: 'follow-up to #97' -> 'follow-up to #90'
Also caught by this rebase onto latest main (docs: align repository
documentation with MVP status, PR #88). The rebase was conflict-free
because PR #88 and the audit modified orthogonal sections of the
shared files (README.md, docs/MVP_RELEASE_STATUS.md, and
docs/PR_CLEANUP_HANDOFF.md).
The rebase confirmed that all five audit findings survived intact:
- core registry has 7 live archetypes
- intelligence registry has 3 archetypes
- plain Product Manager is missing from core
- #90 tracks plain Product Manager
- #91 tracks registry consolidation
No code changes. Docs only. PR #37 and PR #78 untouched.
Validates: pnpm test 12/12, pnpm lint 0 errors, pnpm build 6/6.
---------
Co-authored-by: Cleanup Bot <cleanup-bot@example.com>
1f5adfd to
26120b5
Compare
📝 WalkthroughWalkthroughThis PR adds ChangesCLI color output
Estimated code review effort: 2 (Simple) | ~10 minutes PoemA rabbit hops with chalk in paw, 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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: 1
🧹 Nitpick comments (1)
packages/cli/src/cli.ts (1)
32-50: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueExtract a shared score-color helper.
colorScore,colorDimScore, andcolorBarrepeat the same red/yellow/green threshold logic; a single helper would keep the coloring consistent and easier to update.🤖 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 `@packages/cli/src/cli.ts` around lines 32 - 50, The score coloring logic is duplicated across colorScore, colorDimScore, and colorBar in cli.ts; extract a shared helper that maps a numeric score to the red/yellow/green choice once and reuse it in all three functions. Keep the existing thresholds and formatting behavior, but have colorScore, colorDimScore, and colorBar delegate to the shared helper so updates stay consistent. Use the existing function names as the integration points when refactoring.
🤖 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 `@packages/cli/src/cli.ts`:
- Around line 26-50: Add tests for the new CLI coloring thresholds in
colorScore, colorDimScore, and colorBar. Cover the red/yellow/green bucket
boundaries and the filled/empty bar rendering so the behavior change is
exercised. Place the tests alongside the existing CLI coverage for
packages/cli/src/cli.ts and assert the expected output for values around the
cutoff points.
---
Nitpick comments:
In `@packages/cli/src/cli.ts`:
- Around line 32-50: The score coloring logic is duplicated across colorScore,
colorDimScore, and colorBar in cli.ts; extract a shared helper that maps a
numeric score to the red/yellow/green choice once and reuse it in all three
functions. Keep the existing thresholds and formatting behavior, but have
colorScore, colorDimScore, and colorBar delegate to the shared helper so updates
stay consistent. Use the existing function names as the integration points when
refactoring.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 1966ef47-3ec5-4f80-9eb1-1c62c62255e8
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!**/pnpm-lock.yaml
📒 Files selected for processing (2)
packages/cli/package.jsonpackages/cli/src/cli.ts
| console.error(chalk.red(`Unknown command: ${command}\n`)); | ||
| printHelp(); | ||
| process.exit(1); | ||
| } | ||
| } | ||
|
|
||
| function colorScore(score: number): string { | ||
| if (score <3) return chalk.red(`${score}/5`); | ||
| if (score <4) return chalk.yellow(`${score}/5`); | ||
| return chalk.green(`${score}/5`); | ||
| } | ||
|
|
||
| function colorDimScore(score: number, max: number): string { | ||
| if (score<3) return chalk.red(`${score}/${max}`); | ||
| if (score<4) return chalk.yellow(`${score}/${max}`); | ||
| return chalk.green(`${score}/${max}`); | ||
| } | ||
|
|
||
| function colorBar(score: number, max: number): string { | ||
| const filled = "█".repeat(score); | ||
| const empty = "░".repeat(max - score); | ||
| if (score <3) return chalk.red(filled) + chalk.dim(empty); | ||
| if (score <4) return chalk.yellow(filled) + chalk.dim(empty); | ||
| return chalk.green(filled) + chalk.dim(empty); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
No test coverage for new coloring/threshold logic.
colorScore, colorDimScore, and colorBar introduce new bucketing behavior (red/yellow/green thresholds) but no accompanying tests are visible in this change. As per path instructions, "Behavior changes should be covered by tests" for packages/cli/**.
🤖 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 `@packages/cli/src/cli.ts` around lines 26 - 50, Add tests for the new CLI
coloring thresholds in colorScore, colorDimScore, and colorBar. Cover the
red/yellow/green bucket boundaries and the filled/empty bar rendering so the
behavior change is exercised. Place the tests alongside the existing CLI
coverage for packages/cli/src/cli.ts and assert the expected output for values
around the cutoff points.
Source: Path instructions
|
Hi! @SaharPak, just completed the rebase onto the current main. Here's what was done:
|
Summary
Closes #19
Added color output to the CLI using chalk to make results easier to scan.
Changes
Screenshots
Summary by CodeRabbit
New Features
Bug Fixes