fix: standardize UI box width calculations across components#1305
Conversation
🦋 Changeset detectedLatest commit: 3fe999c The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughIntroduces a shared getBoxWidth helper in CLI UI utilities and applies it across warning/success/info/error displays, task table, and two UI components to standardize box widths. Adds tests for width calculation logic. Includes a changeset entry for a patch release note. No public API removals. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User as User
participant CLI as CLI Command
participant UI as UI Components
participant Util as getBoxWidth()
participant Boxen as boxen
User->>CLI: Execute command
CLI->>UI: Render sections (warning/info/table/next task)
UI->>Util: Request width (percentage, minWidth)
Util-->>UI: Computed width (from terminal or fallback)
UI->>Boxen: Render box with width
Boxen-->>User: Display aligned boxes
note over Util,UI: Centralized width logic replaces per-component calculations
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Pre-merge checks and finishing touches✅ 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
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
.changeset/fix-warning-box-alignment.md(1 hunks)apps/cli/src/utils/ui.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
.changeset/*.md
📄 CodeRabbit inference engine (.cursor/rules/changeset.mdc)
.changeset/*.md: When runningnpm run changesetornpx changeset add, provide a concise summary of the changes for theCHANGELOG.mdin imperative mood, typically a single line, and not a detailed Git commit message.
The changeset summary should be user-facing, describing what changed in the released version that is relevant to users or consumers of the package.
Do not use your detailed Git commit message body as the changeset summary.
Files:
.changeset/fix-warning-box-alignment.md
.changeset/*
📄 CodeRabbit inference engine (.cursor/rules/new_features.mdc)
Create appropriate changesets for new features, use semantic versioning, include tagged system information in release notes, and document breaking changes if any.
Files:
.changeset/fix-warning-box-alignment.md
.changeset/**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
Changeset entries should be user-facing, describing the end-user impact rather than code specifics
Files:
.changeset/fix-warning-box-alignment.md
🪛 markdownlint-cli2 (0.18.1)
.changeset/fix-warning-box-alignment.md
5-5: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
🔇 Additional comments (2)
.changeset/fix-warning-box-alignment.md (1)
1-5: LGTM! Changeset follows conventions.The changeset correctly documents a patch release with a concise, user-facing description in imperative mood as per coding guidelines.
Note: The markdownlint warning about missing a top-level heading (MD041) is a false positive—changeset files follow their own format and don't require markdown headings.
apps/cli/src/utils/ui.ts (1)
221-222: Verify if other message functions need the same width treatment.The width constraint now aligns the warning box with dashboard boxes. However,
displayError(lines 178-191),displaySuccess(lines 196-207), anddisplayInfo(lines 230-238) remain unconstrained, which may lead to visual inconsistency when different message types appear in the same session.Was it intentional to update only
displayWarning, or should the other message functions also adopt the same width calculation for comprehensive UI consistency?If other message functions should match, apply similar changes:
// In displayError, displaySuccess, and displayInfo const boxWidth = Math.floor((process.stdout.columns || 80) * 0.9); // Then add to boxen options: width: boxWidth
1251b96 to
87356bd
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (24)
.changeset/auto-update-changelog-highlights.md(0 hunks).changeset/fix-warning-box-alignment.md(1 hunks).changeset/mean-planes-wave.md(0 hunks).changeset/nice-ways-hope.md(0 hunks).changeset/plain-falcons-serve.md(0 hunks).changeset/pre.json(0 hunks).changeset/silent-bushes-grow.md(0 hunks).changeset/smart-owls-relax.md(0 hunks)CHANGELOG.md(1 hunks)apps/cli/CHANGELOG.md(1 hunks)apps/cli/src/ui/components/next-task.component.ts(2 hunks)apps/cli/src/ui/components/suggested-steps.component.ts(2 hunks)apps/cli/src/utils/ui.ts(7 hunks)apps/docs/CHANGELOG.md(1 hunks)apps/docs/package.json(1 hunks)apps/extension/CHANGELOG.md(1 hunks)apps/extension/package.json(1 hunks)package.json(1 hunks)packages/ai-sdk-provider-grok-cli/CHANGELOG.md(1 hunks)packages/build-config/CHANGELOG.md(1 hunks)packages/claude-code-plugin/CHANGELOG.md(1 hunks)packages/claude-code-plugin/package.json(1 hunks)packages/tm-core/CHANGELOG.md(1 hunks)tests/unit/cli-ui.test.js(1 hunks)
💤 Files with no reviewable changes (7)
- .changeset/smart-owls-relax.md
- .changeset/silent-bushes-grow.md
- .changeset/plain-falcons-serve.md
- .changeset/auto-update-changelog-highlights.md
- .changeset/nice-ways-hope.md
- .changeset/mean-planes-wave.md
- .changeset/pre.json
🧰 Additional context used
📓 Path-based instructions (14)
tests/{unit,integration,e2e,fixtures}/**/*.js
📄 CodeRabbit inference engine (.cursor/rules/architecture.mdc)
Test files must be organized as follows: unit tests in tests/unit/, integration tests in tests/integration/, end-to-end tests in tests/e2e/, and test fixtures in tests/fixtures/.
Files:
tests/unit/cli-ui.test.js
tests/unit/*.js
📄 CodeRabbit inference engine (.cursor/rules/architecture.mdc)
Each module should have a corresponding unit test file in tests/unit/ that reflects the module structure (one test file per module).
Files:
tests/unit/cli-ui.test.js
**/*.{test,spec}.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/git_workflow.mdc)
**/*.{test,spec}.{js,ts,jsx,tsx}: Create a test file and ensure all tests pass when all subtasks are complete; commit tests if added or modified
When all subtasks are complete, run final testing using the appropriate test runner (e.g., npm test, jest, or manual testing)
Files:
tests/unit/cli-ui.test.js
**/*.test.js
📄 CodeRabbit inference engine (.cursor/rules/tests.mdc)
**/*.test.js: Never use asynchronous operations in tests. Make all mocks return synchronous values when possible.
Always mock tests properly based on the way the tested functions are defined and used.
Follow the test file organization: mocks must be set up before importing modules under test, and spies on mocked modules should be set up after imports.
Use fixtures from tests/fixtures/ for consistent sample data across tests.
Always declare mocks before importing the modules being tested in Jest test files.
Use jest.spyOn() after imports to create spies on mock functions and reference these spies in test assertions.
When testing functions with callbacks, get the callback from your mock's call arguments, execute it directly with test inputs, and verify the results.
For ES modules, use jest.mock() before static imports and jest.unstable_mockModule() before dynamic imports to mock dependencies.
Reset mock functions (mockFn.mockReset()) before dynamic imports if they might have been called previously.
When verifying console assertions, assert against the actual arguments passed (single formatted string), not multiple arguments.
Use mock-fs to mock file system operations in tests, and restore the file system after each test.
Mock API calls (e.g., Anthropic/Claude) by mocking the entire module and providing predictable responses.
Set mock environment variables in test setup and restore them after each test.
Maintain test fixtures separate from test logic.
Follow the mock-first-then-import pattern for all Jest mocks.
Do not define mock variables before jest.mock() calls (they won't be accessible due to hoisting).
Use test-specific file paths (e.g., 'test-tasks.json') for all file operations in tests.
Mock readJSON and writeJSON to avoid real file system interactions in tests.
Verify file operations use the correct paths in expect statements.
Use different file paths for each test to avoid test interdependence.
Verify modifications on the in-memory task objects passed to w...
Files:
tests/unit/cli-ui.test.js
tests/unit/**/*.test.js
📄 CodeRabbit inference engine (.cursor/rules/tests.mdc)
tests/unit/**/*.test.js: Unit tests must be located in tests/unit/, test individual functions and utilities in isolation, mock all external dependencies, and keep tests small, focused, and fast.
Do not include actual command execution in unit tests.
Files:
tests/unit/cli-ui.test.js
tests/{unit,integration,e2e}/**/*.test.js
📄 CodeRabbit inference engine (.cursor/rules/tests.mdc)
tests/{unit,integration,e2e}/**/*.test.js: When testing CLI commands built with Commander.js, test the command action handlers directly rather than trying to mock the entire Commander.js chain.
When mocking the Commander.js chain, mock ALL chainable methods (option, argument, action, on, etc.) and return this (or the mock object) from all chainable method mocks.
Explicitly handle all options, including defaults and shorthand flags (e.g., -p for --prompt), and include null/undefined checks in test implementations for parameters that might be optional.
Do not try to use the real action implementation without proper mocking, and do not mock Commander partially—either mock it completely or test the action directly.
Mock the action handlers for CLI commands and verify they're called with correct arguments.
Use sample task fixtures for consistent test data, mock file system operations, and test both success and error paths for task operations.
Mock console output and verify correct formatting in UI function tests. Use flexible assertions like toContain() or toMatch() for formatted output.
Mock chalk functions to return the input text to make testing easier while still verifying correct function calls.
Files:
tests/unit/cli-ui.test.js
**/*.js
📄 CodeRabbit inference engine (.cursor/rules/tests.mdc)
**/*.js: Declare and initialize global variables at the top of modules to avoid hoisting issues.
Use proper function declarations to avoid hoisting issues and initialize variables before they are referenced.
Do not reference variables before their declaration in module scope.
Use dynamic imports (import()) to avoid initialization order issues in modules.
Files:
tests/unit/cli-ui.test.js
**/*.{test,spec}.*
📄 CodeRabbit inference engine (.cursor/rules/test_workflow.mdc)
Test files should follow naming conventions: .test., .spec., or _test. depending on the language
Files:
tests/unit/cli-ui.test.js
tests/{unit,integration,e2e}/**
📄 CodeRabbit inference engine (.cursor/rules/test_workflow.mdc)
Organize test directories by test type (unit, integration, e2e) and mirror source structure where possible
Files:
tests/unit/cli-ui.test.js
**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{test,spec}.{js,jsx,ts,tsx}: NEVER use async/await in test functions unless testing actual asynchronous operations
Use synchronous top-level imports in tests; avoid dynamic await import()
Keep test bodies synchronous whenever possible
Files:
tests/unit/cli-ui.test.js
.changeset/*.md
📄 CodeRabbit inference engine (.cursor/rules/changeset.mdc)
.changeset/*.md: When runningnpm run changesetornpx changeset add, provide a concise summary of the changes for theCHANGELOG.mdin imperative mood, typically a single line, and not a detailed Git commit message.
The changeset summary should be user-facing, describing what changed in the released version that is relevant to users or consumers of the package.
Do not use your detailed Git commit message body as the changeset summary.
Files:
.changeset/fix-warning-box-alignment.md
.changeset/*
📄 CodeRabbit inference engine (.cursor/rules/new_features.mdc)
Create appropriate changesets for new features, use semantic versioning, include tagged system information in release notes, and document breaking changes if any.
Files:
.changeset/fix-warning-box-alignment.md
.changeset/**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
Changeset entries should be user-facing, describing the end-user impact rather than code specifics
Files:
.changeset/fix-warning-box-alignment.md
package.json
📄 CodeRabbit inference engine (.cursor/rules/test_workflow.mdc)
Add and update test scripts in package.json to include test, test:watch, test:coverage, test:unit, test:integration, test:e2e, and test:ci
Files:
package.json
🧠 Learnings (7)
📚 Learning: 2025-09-26T19:03:33.225Z
Learnt from: Crunchyman-ralph
PR: eyaltoledano/claude-task-master#1252
File: package.json:130-132
Timestamp: 2025-09-26T19:03:33.225Z
Learning: In the eyaltoledano/claude-task-master repository, all packages with tm prefix are internal packages that are part of the monorepo structure and are not published externally.
Applied to files:
packages/claude-code-plugin/package.json
📚 Learning: 2025-09-26T19:05:47.555Z
Learnt from: Crunchyman-ralph
PR: eyaltoledano/claude-task-master#1252
File: packages/ai-sdk-provider-grok-cli/package.json:11-13
Timestamp: 2025-09-26T19:05:47.555Z
Learning: In the eyaltoledano/claude-task-master repository, internal tm/ packages use a specific export pattern where the "exports" field points to TypeScript source files (./src/index.ts) while "main" points to compiled output (./dist/index.js) and "types" points to source files (./src/index.ts). This pattern is used consistently across internal packages like tm/core and tm/ai-sdk-provider-grok-cli because they are consumed directly during build-time bundling with tsdown rather than being published as separate packages.
Applied to files:
packages/claude-code-plugin/package.json
📚 Learning: 2025-09-22T19:45:13.323Z
Learnt from: Crunchyman-ralph
PR: eyaltoledano/claude-task-master#1232
File: packages/build-config/package.json:14-15
Timestamp: 2025-09-22T19:45:13.323Z
Learning: In the eyaltoledano/claude-task-master repository, Crunchyman-ralph intentionally omits version fields from internal packages (like tm/build-config) to prevent changesets from releasing new versions for these packages. This is the desired behavior for internal tooling packages that should not be published or versioned independently.
Applied to files:
packages/claude-code-plugin/package.json
📚 Learning: 2025-07-18T17:09:16.839Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/context_gathering.mdc:0-0
Timestamp: 2025-07-18T17:09:16.839Z
Learning: Display token breakdowns using a detailed, sectioned format, showing tokens per task, file, and prompt, and use formatting utilities like `boxen` and `chalk` for CLI output.
Applied to files:
apps/cli/src/ui/components/next-task.component.tsapps/cli/src/ui/components/suggested-steps.component.ts
📚 Learning: 2025-07-31T22:07:49.716Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/commands.mdc:0-0
Timestamp: 2025-07-31T22:07:49.716Z
Learning: Applies to scripts/modules/commands.js : Use boxen for important success messages with clear formatting, provide suggested next steps after command completion, and include ready-to-use commands for follow-up actions.
Applied to files:
apps/cli/src/ui/components/suggested-steps.component.tsapps/cli/src/utils/ui.ts
📚 Learning: 2025-07-18T17:16:32.622Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/ui.mdc:0-0
Timestamp: 2025-07-18T17:16:32.622Z
Learning: Applies to scripts/modules/ui.js : Provide next step suggestions after command completion using a consistent format
Applied to files:
apps/cli/src/ui/components/suggested-steps.component.ts
📚 Learning: 2025-07-18T17:16:13.793Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/tests.mdc:0-0
Timestamp: 2025-07-18T17:16:13.793Z
Learning: Applies to tests/{unit,integration,e2e}/**/*.test.js : Mock console output and verify correct formatting in UI function tests. Use flexible assertions like toContain() or toMatch() for formatted output.
Applied to files:
tests/unit/cli-ui.test.js
🧬 Code graph analysis (3)
apps/cli/src/ui/components/next-task.component.ts (1)
apps/cli/src/utils/ui.ts (1)
getBoxWidth(135-141)
apps/cli/src/ui/components/suggested-steps.component.ts (1)
apps/cli/src/utils/ui.ts (1)
getBoxWidth(135-141)
tests/unit/cli-ui.test.js (1)
apps/cli/src/utils/ui.ts (1)
getBoxWidth(135-141)
🪛 LanguageTool
packages/claude-code-plugin/CHANGELOG.md
[grammar] ~1-~1: Hier könnte ein Fehler sein.
Context: # @tm/claude-code-plugin ## 0.0.2
(QB_NEW_DE)
[grammar] ~3-~3: Hier könnte ein Fehler sein.
Context: # @tm/claude-code-plugin ## 0.0.2
(QB_NEW_DE)
apps/docs/CHANGELOG.md
[grammar] ~3-~3: Hier könnte ein Fehler sein.
Context: # docs ## 0.0.6 ## 0.0.5 ## 0.0.4 ## 0.0.3 ## 0.0.2 ## ...
(QB_NEW_DE)
packages/tm-core/CHANGELOG.md
[grammar] ~7-~7: Use correct spacing
Context: # Changelog ## null ## null ## null ## 0.26.1 All notable changes to the @task...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
.changeset/fix-warning-box-alignment.md
[grammar] ~5-~5: There might be a mistake here.
Context: ...rd box width for consistent UI alignment
(QB_NEW_EN_OTHER)
apps/cli/CHANGELOG.md
[grammar] ~14-~14: There might be a mistake here.
Context: ... ## null ### Patch Changes - Updated dependencies []: - @tm/core@null ## null ### Patch C...
(QB_NEW_EN_OTHER)
[grammar] ~15-~15: Use correct spacing
Context: ...dated dependencies []: - @tm/core@null ## null ### Patch Changes - Updated depen...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~17-~17: Use correct spacing
Context: ...endencies []: - @tm/core@null ## null ### Patch Changes - Updated dependencies []...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~19-~19: Use correct spacing
Context: ...tm/core@null ## null ### Patch Changes - Updated dependencies []: - @tm/core@nu...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
packages/build-config/CHANGELOG.md
[grammar] ~7-~7: Hier könnte ein Fehler sein.
Context: .../build-config ## null ## null ## null ## 1.0.1
(QB_NEW_DE)
apps/extension/CHANGELOG.md
[grammar] ~3-~3: Use correct spacing
Context: # Change Log ## 0.25.6 ## 0.25.6-rc.0 ### Patch Changes - Update...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
🪛 markdownlint-cli2 (0.18.1)
packages/ai-sdk-provider-grok-cli/CHANGELOG.md
5-5: Multiple headings with the same content
(MD024, no-duplicate-heading)
packages/tm-core/CHANGELOG.md
7-7: Multiple headings with the same content
(MD024, no-duplicate-heading)
.changeset/fix-warning-box-alignment.md
5-5: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
apps/cli/CHANGELOG.md
17-17: Multiple headings with the same content
(MD024, no-duplicate-heading)
packages/build-config/CHANGELOG.md
7-7: Multiple headings with the same content
(MD024, no-duplicate-heading)
CHANGELOG.md
29-29: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Typecheck
🔇 Additional comments (2)
apps/cli/src/utils/ui.ts (2)
129-141: Excellent implementation addressing previous feedback!The
getBoxWidthhelper successfully centralizes terminal width calculations with sensible defaults and defensive programming:
- Gracefully handles undefined
process.stdout.columnswith 80-column fallback- Enforces minimum width via
Math.maxto prevent unusably small boxes- Provides configurable percentage and minimum width parameters
- Clear JSDoc documentation
This directly addresses the previous review comment about extracting a shared helper function and eliminates the inconsistencies that existed across the codebase.
193-194: Consistent application of shared helper across display functions.All display functions (
displayError,displaySuccess,displayWarning,displayInfo) now consistently usegetBoxWidth()with default parameters (90% width, 40 minimum) and pass the calculated width to boxen. This ensures uniform box sizing across error, success, warning, and info messages.Also applies to: 214-215, 233-234, 249-250
- Calculate warning box width as 90% of terminal width - Matches dashboard box width calculation for consistent UI - Fixes misaligned warning message when no tasks found Resolves UI alignment issue where empty state warning box was narrower than dashboard boxes
Extract shared getBoxWidth() helper function to ensure consistent box width calculations across all UI components. This addresses CodeRabbit's review comment about eliminating duplication between displayWarning() and table width calculations. Changes: - Export getBoxWidth() helper from ui.ts with configurable percentage and minimum width - Update displayWarning() to use getBoxWidth() - Update createTaskTable() to use getBoxWidth() for consistent table widths - Update displayRecommendedNextTask() to use getBoxWidth() - Update displaySuggestedNextSteps() to use getBoxWidth() This ensures all boxes maintain consistent width calculations and handle edge cases (like undefined process.stdout.columns) uniformly.
Add unit tests for the getBoxWidth() helper function to ensure correct behavior across various scenarios including edge cases. Tests cover: - Default parameter usage (0.9 percentage, 40 min width) - Custom percentage and minimum width values - Terminal width edge cases (undefined, 0, very large) - Minimum width enforcement when terminal is too narrow - Floor operation on calculated widths - Consistency across multiple calls - Real-world usage scenarios from displayWarning(), createTaskTable(), and displayRecommendedNextTask() All 16 tests pass successfully.
Extend getBoxWidth() usage to displayError(), displaySuccess(), and displayInfo() functions to ensure comprehensive UI consistency across all message types. This addresses CodeRabbit's suggestion to prevent visual inconsistency when different message types appear in the same session. Changes: - displayError() now uses getBoxWidth() - displaySuccess() now uses getBoxWidth() - displayInfo() now uses getBoxWidth() All message boxes (error, success, warning, info) now maintain consistent width calculations, ensuring uniform appearance regardless of message type.
87356bd to
3e3c654
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
apps/cli/src/utils/ui.spec.ts(1 hunks)apps/cli/src/utils/ui.ts(7 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{test,spec}.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/git_workflow.mdc)
**/*.{test,spec}.{js,ts,jsx,tsx}: Create a test file and ensure all tests pass when all subtasks are complete; commit tests if added or modified
When all subtasks are complete, run final testing using the appropriate test runner (e.g., npm test, jest, or manual testing)
Files:
apps/cli/src/utils/ui.spec.ts
**/*.{test,spec}.*
📄 CodeRabbit inference engine (.cursor/rules/test_workflow.mdc)
Test files should follow naming conventions: .test., .spec., or _test. depending on the language
Files:
apps/cli/src/utils/ui.spec.ts
**/?(*.)+(spec|test).ts
📄 CodeRabbit inference engine (.cursor/rules/test_workflow.mdc)
In JavaScript/TypeScript projects using Jest, test files should match *.test.ts and *.spec.ts patterns
Files:
apps/cli/src/utils/ui.spec.ts
**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{test,spec}.{js,jsx,ts,tsx}: NEVER use async/await in test functions unless testing actual asynchronous operations
Use synchronous top-level imports in tests; avoid dynamic await import()
Keep test bodies synchronous whenever possible
Files:
apps/cli/src/utils/ui.spec.ts
🧠 Learnings (3)
📚 Learning: 2025-07-18T17:16:13.793Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/tests.mdc:0-0
Timestamp: 2025-07-18T17:16:13.793Z
Learning: Applies to tests/{unit,integration,e2e}/**/*.test.js : Mock console output and verify correct formatting in UI function tests. Use flexible assertions like toContain() or toMatch() for formatted output.
Applied to files:
apps/cli/src/utils/ui.spec.ts
📚 Learning: 2025-07-31T22:07:49.716Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/commands.mdc:0-0
Timestamp: 2025-07-31T22:07:49.716Z
Learning: Applies to scripts/modules/commands.js : Use boxen for important success messages with clear formatting, provide suggested next steps after command completion, and include ready-to-use commands for follow-up actions.
Applied to files:
apps/cli/src/utils/ui.ts
📚 Learning: 2025-07-18T17:16:32.622Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/ui.mdc:0-0
Timestamp: 2025-07-18T17:16:32.622Z
Learning: Applies to scripts/modules/ui.js : Use cli-table3 for table rendering, include colored bold headers, and set appropriate column widths for readability
Applied to files:
apps/cli/src/utils/ui.ts
🧬 Code graph analysis (1)
apps/cli/src/utils/ui.spec.ts (1)
apps/cli/src/utils/ui.ts (1)
getBoxWidth(135-141)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Test
🔇 Additional comments (7)
apps/cli/src/utils/ui.spec.ts (2)
15-35: Robust columns mocking setup looks goodGetter spy approach is solid and restores cleanly across tests. No issues.
49-156: Great coverage of defaults, flooring, min enforcement, undefined/zero widthsTests validate core behavior and key integration scenarios (warning, table, recommended task) and repeated-call consistency. Nicely aligned with our test guidelines.
As per coding guidelines
apps/cli/src/utils/ui.ts (5)
193-205: Adoption of getBoxWidth in displayError is correctWidth is now centralized and passed to boxen. LGTM.
214-224: Success message uses centralized width correctlyUniform sizing via getBoxWidth. LGTM.
233-241: Warning message width standardizedConsistent implementation with other display helpers. LGTM.
249-257: Info message width standardizedConsistent with other helpers. LGTM.
311-328: Table now derives widths from shared helperUsing getBoxWidth(0.9, 100) is appropriate; renaming to tableWidth improves clarity. LGTM.
| it('should consistently return same value for same inputs', () => { | ||
| columnsSpy.mockReturnValue(100); | ||
| const width1 = getBoxWidth(0.9, 40); | ||
| const width2 = getBoxWidth(0.9, 40); | ||
| const width3 = getBoxWidth(0.9, 40); | ||
| expect(width1).toBe(width2); | ||
| expect(width2).toBe(width3); | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Add tests for invalid inputs (negative/NaN percentages, negative minWidth)
Guard against edge inputs to lock behavior. Suggest adding the following tests to document and enforce contracts alongside the helper hardening suggested in ui.ts.
@@
it('should consistently return same value for same inputs', () => {
columnsSpy.mockReturnValue(100);
const width1 = getBoxWidth(0.9, 40);
const width2 = getBoxWidth(0.9, 40);
const width3 = getBoxWidth(0.9, 40);
expect(width1).toBe(width2);
expect(width2).toBe(width3);
});
+
+ it('should clamp percentage to [0,1] and default on NaN', () => {
+ columnsSpy.mockReturnValue(100);
+ // < 0 => clamp to 0
+ expect(getBoxWidth(-0.5, 40)).toBe(40);
+ // > 1 => clamp to 1
+ expect(getBoxWidth(1.5, 40)).toBe(100);
+ // NaN => default to 0.9
+ // @ts-expect-error intentional invalid input for test
+ expect(getBoxWidth(Number.NaN, 40)).toBe(90);
+ });
+
+ it('should sanitize negative minWidth to 0', () => {
+ columnsSpy.mockReturnValue(100);
+ expect(getBoxWidth(0.9, -10)).toBe(90);
+ });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it('should consistently return same value for same inputs', () => { | |
| columnsSpy.mockReturnValue(100); | |
| const width1 = getBoxWidth(0.9, 40); | |
| const width2 = getBoxWidth(0.9, 40); | |
| const width3 = getBoxWidth(0.9, 40); | |
| expect(width1).toBe(width2); | |
| expect(width2).toBe(width3); | |
| }); | |
| it('should consistently return same value for same inputs', () => { | |
| columnsSpy.mockReturnValue(100); | |
| const width1 = getBoxWidth(0.9, 40); | |
| const width2 = getBoxWidth(0.9, 40); | |
| const width3 = getBoxWidth(0.9, 40); | |
| expect(width1).toBe(width2); | |
| expect(width2).toBe(width3); | |
| }); | |
| it('should clamp percentage to [0,1] and default on NaN', () => { | |
| columnsSpy.mockReturnValue(100); | |
| // < 0 => clamp to 0 | |
| expect(getBoxWidth(-0.5, 40)).toBe(40); | |
| // > 1 => clamp to 1 | |
| expect(getBoxWidth(1.5, 40)).toBe(100); | |
| // NaN => default to 0.9 | |
| // @ts-expect-error intentional invalid input for test | |
| expect(getBoxWidth(Number.NaN, 40)).toBe(90); | |
| }); | |
| it('should sanitize negative minWidth to 0', () => { | |
| columnsSpy.mockReturnValue(100); | |
| expect(getBoxWidth(0.9, -10)).toBe(90); | |
| }); |
🤖 Prompt for AI Agents
In apps/cli/src/utils/ui.spec.ts around lines 149 to 156, add unit tests that
cover invalid inputs: call getBoxWidth with a negative percentage, with NaN as
the percentage, and with a negative minWidth, and assert the function rejects
these invalid inputs (e.g. throws an Error or TypeError) so the contract is
documented and enforced; keep the new tests grouped with the existing
"consistent return" test and mirror its style (use columnsSpy.mockReturnValue to
control terminal width) so failures are deterministic.
| /** | ||
| * Calculate box width as percentage of terminal width | ||
| * @param percentage - Percentage of terminal width to use (default: 0.9) | ||
| * @param minWidth - Minimum width to enforce (default: 40) | ||
| * @returns Calculated box width | ||
| */ | ||
| export function getBoxWidth( | ||
| percentage: number = 0.9, | ||
| minWidth: number = 40 | ||
| ): number { | ||
| const terminalWidth = process.stdout.columns || 80; | ||
| return Math.max(Math.floor(terminalWidth * percentage), minWidth); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Verify there are no leftover ad-hoc width calculations
Run a quick scan to confirm all width logic routes through getBoxWidth and no direct process.stdout.columns math remains elsewhere.
Also applies to: 193-205, 214-224, 233-241, 249-257, 311-328
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "1) Occurrences of process.stdout.columns:"
rg -n -C2 --type=ts --type=js '\bprocess\.stdout\.columns\b' || true
echo
echo "2) Potential ad-hoc width calcs using columns + Math.floor:"
rg -n -P -C2 'Math\.floor\([^)]*columns[^)]*\)' || true
echo
echo "3) Call sites of getBoxWidth:"
rg -n 'getBoxWidth\(' || true
echo
echo "4) boxen() calls without explicit width option (for review):"
rg -n -P -C2 'boxen\([^)]*\)\s*[,)]' || trueLength of output: 23062
Replace ad-hoc width calculations with getBoxWidth
Multiple components still compute width via process.stdout.columns:
- apps/cli/src/ui/components/task-detail.component.ts (4×
process.stdout.columns * 0.95 || 100) - apps/cli/src/commands/start.command.ts (2×
process.stdout.columns * 0.95 || 100) - apps/cli/src/ui/components/dashboard.component.ts (
process.stdout.columns || 80)
UsegetBoxWidth(percentage, minWidth?)instead to centralize width logic.
🤖 Prompt for AI Agents
In apps/cli/src/utils/ui.ts around lines 129 to 141, you added
getBoxWidth(percentage,minWidth) to centralize terminal width logic; now update
all places that use ad-hoc calculations to call this helper instead: replace
occurrences of "process.stdout.columns * 0.95 || 100" with getBoxWidth(0.95,
100) in apps/cli/src/ui/components/task-detail.component.ts and
apps/cli/src/commands/start.command.ts, and replace "process.stdout.columns ||
80" with getBoxWidth(undefined, 80) (or getBoxWidth(0.9, 80) if a specific
percentage is desired) in apps/cli/src/ui/components/dashboard.component.ts so
all width calculations use the shared function and preserve previous fallback
minimums.
| export function getBoxWidth( | ||
| percentage: number = 0.9, | ||
| minWidth: number = 40 | ||
| ): number { | ||
| const terminalWidth = process.stdout.columns || 80; | ||
| return Math.max(Math.floor(terminalWidth * percentage), minWidth); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Harden getBoxWidth: sanitize inputs and clamp percentage
Current logic assumes sane inputs. Clamp percentage to [0,1], coerce/validate columns, and sanitize minWidth to a non-negative number to avoid surprises on invalid values.
-export function getBoxWidth(
- percentage: number = 0.9,
- minWidth: number = 40
-): number {
- const terminalWidth = process.stdout.columns || 80;
- return Math.max(Math.floor(terminalWidth * percentage), minWidth);
-}
+export function getBoxWidth(
+ percentage: number = 0.9,
+ minWidth: number = 40
+): number {
+ const rawCols = Number((process.stdout as any)?.columns);
+ const terminalWidth =
+ Number.isFinite(rawCols) && rawCols > 0 ? rawCols : 80;
+ const pct =
+ Number.isFinite(percentage) ? Math.min(Math.max(percentage, 0), 1) : 0.9;
+ const min =
+ Number.isFinite(minWidth) ? Math.max(0, minWidth) : 40;
+ return Math.max(Math.floor(terminalWidth * pct), min);
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function getBoxWidth( | |
| percentage: number = 0.9, | |
| minWidth: number = 40 | |
| ): number { | |
| const terminalWidth = process.stdout.columns || 80; | |
| return Math.max(Math.floor(terminalWidth * percentage), minWidth); | |
| } | |
| export function getBoxWidth( | |
| percentage: number = 0.9, | |
| minWidth: number = 40 | |
| ): number { | |
| const rawCols = Number((process.stdout as any)?.columns); | |
| const terminalWidth = | |
| Number.isFinite(rawCols) && rawCols > 0 ? rawCols : 80; | |
| const pct = | |
| Number.isFinite(percentage) ? Math.min(Math.max(percentage, 0), 1) : 0.9; | |
| const min = | |
| Number.isFinite(minWidth) ? Math.max(0, minWidth) : 40; | |
| return Math.max(Math.floor(terminalWidth * pct), min); | |
| } |
🤖 Prompt for AI Agents
In apps/cli/src/utils/ui.ts around lines 135 to 141, getBoxWidth currently
trusts inputs; sanitize and clamp them: coerce percentage to a number and clamp
to the [0,1] range, coerce process.stdout.columns to an integer >= 0 (fall back
to 80 if missing or invalid), coerce minWidth to a non-negative integer (use 0
if invalid), then compute width as Math.max(Math.floor(terminalWidth *
percentage), minWidth) and return that value.
The "NO TASKS AVAILABLE" message box was missing width and fullscreen properties, causing misalignment with other UI elements. Added consistent styling to match the "RECOMMENDED NEXT TASK" box by: - Adding getBoxWidth(0.97) to match terminal width behavior - Adding fullscreen: false for consistent rendering - Importing getBoxWidth utility in next.command.ts This ensures all task-related boxes maintain consistent width and alignment across different terminal sizes. Fixes: eyaltoledano#1305
Summary
Fixes visual inconsistencies in UI box width calculations across multiple components. This PR addresses the original warning box alignment issue and extends the fix to ensure all UI boxes use consistent width calculations.
Changes Made
Core Change
getBoxWidth()helper function inapps/cli/src/utils/ui.tsprocess.stdout.columnsundefined edge case gracefullyComponents Updated
displayWarning()- Now usesgetBoxWidth()instead of no width specificationcreateTaskTable()- Updated table width to usegetBoxWidth(0.9, 100)for consistencydisplayRecommendedNextTask()- Fixed misalignment when task status is "in-progress"displaySuggestedNextSteps()- Updated to use shared helperTechnical Details
All components now use the shared
getBoxWidth()helper:This ensures:
Issues Fixed
Testing
Tested on macOS with:
Screenshots
Before:
After:
Risk Assessment
Low - Refactoring only, no behavioral changes beyond consistent width calculations
Performance Considerations
No performance impact - simply consolidates existing calculations into a shared helper function.
Deployment Notes
None required - pure UI cosmetic improvements.
Fixes
Fixes #1304
Summary by CodeRabbit
Bug Fixes
Style