Skip to content

fix(cli): use the plotext plotsize API - #1588

Open
xianlubird wants to merge 1 commit into
ai-dynamo:mainfrom
xianlubird:fix/plotext-plotsize
Open

fix(cli): use the plotext plotsize API#1588
xianlubird wants to merge 1 commit into
ai-dynamo:mainfrom
xianlubird:fix/plotext-plotsize

Conversation

@xianlubird

@xianlubird xianlubird commented Aug 24, 2026

Copy link
Copy Markdown

Overview:

Fix terminal Pareto rendering failures caused by calling a Plotext API alias that is not available in every installed environment.

Details:

  • Use the canonical plotsize API when setting terminal plot dimensions.
  • Extend the existing plain-output regression test to assert the expected 80 by 30 dimensions.

Where should the reviewer start?

  • src/aiconfigurator/sdk/pareto_analysis.py
  • tests/unit/cli/test_plain_output.py

Related Issues:

Validation:

  • git diff --check
  • Plotext 5.3.2 API smoke check passed.
  • Full test suite was not run.

Summary by CodeRabbit

  • Bug Fixes

    • Fixed ASCII Pareto chart rendering by updating plot sizing to use the supported plotting interface.
    • Ensured consistent 80×30 chart dimensions for both ANSI and plain-text output modes.
  • Tests

    • Added coverage verifying Pareto output sizing across supported display formats.

Signed-off-by: xianlubird <xianlubird@gmail.com>
@xianlubird
xianlubird requested review from a team as code owners August 24, 2026 10:23
@copy-pr-bot

copy-pr-bot Bot commented Aug 24, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@github-actions github-actions Bot added the fix label Aug 24, 2026
@coderabbitai

coderabbitai Bot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 17d1e29f-6058-4f8d-abcb-83d4c59deb6b

📥 Commits

Reviewing files that changed from the base of the PR and between 2dc2406 and dab72c1.

📒 Files selected for processing (2)
  • src/aiconfigurator/sdk/pareto_analysis.py
  • tests/unit/cli/test_plain_output.py

Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review.

📜 Recent review details
⏰ Context from checks skipped due to timeout. (13)
  • GitHub Check: Collect snapshot (old)
  • GitHub Check: Collect snapshot (new)
  • GitHub Check: Python 3.13 compatibility
  • GitHub Check: Python 3.11 compatibility
  • GitHub Check: Python 3.12 compatibility
  • GitHub Check: Build and Test (e2e)
  • GitHub Check: aic-core public API contract
  • GitHub Check: Build and Test (unit)
  • GitHub Check: Cargo Deny
  • GitHub Check: Build wheels (manylinux_2_28_aarch64)
  • GitHub Check: Build wheels (macosx_arm64)
  • GitHub Check: Rust/Python engine-step parity
  • GitHub Check: Build wheels (manylinux_2_28_x86_64)
🧰 Additional context used
📓 Path-based instructions (3)
**/*

⚙️ CodeRabbit configuration file

**/*: - Prefer applicable inline comments. When the correct fix is clear, small, and limited to the commented diff hunk, include it as a GitHub Suggested Change so the author can apply it with one click.

  • Do not use a suggested change when the fix requires broader design choices, multiple files, generated artifacts, unavailable context, or validation that cannot be inferred from the diff.
  • If a comment is not directly applicable, state the smallest concrete next step and why a one-click suggestion is not safe.

Files:

  • tests/unit/cli/test_plain_output.py
  • src/aiconfigurator/sdk/pareto_analysis.py
tests/**

⚙️ CodeRabbit configuration file

tests/**: - Check that tests cover the changed behavior rather than only the happy path.

  • Watch for fixtures or golden outputs that mask backend drift, support-matrix ordering changes, or CLI output regressions.

Files:

  • tests/unit/cli/test_plain_output.py
src/aiconfigurator/sdk/**

⚙️ CodeRabbit configuration file

src/aiconfigurator/sdk/**: - Verify SDK API changes remain compatible with generator inputs, profiler data flow, and documented examples.

  • Flag silent schema or field-name drift between SDK models and generator/module bridge code.

Files:

  • src/aiconfigurator/sdk/pareto_analysis.py
🔇 Additional comments (3)
src/aiconfigurator/sdk/pareto_analysis.py (1)

1725-1725: LGTM!

tests/unit/cli/test_plain_output.py (2)

13-13: LGTM!


80-90: LGTM!


Walkthrough

The Pareto output code now uses the current plotext.plotsize API. The ANSI and plain output test patches this API and verifies an 80-by-30 canvas.

Changes

Pareto plot sizing

Layer / File(s) Summary
Update plot sizing and validation
src/aiconfigurator/sdk/pareto_analysis.py, tests/unit/cli/test_plain_output.py
draw_pareto_to_string calls plotext.plotsize(80, 30). The parameterized test verifies this call for ANSI and plain output modes.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Merge Risk: ⚪ Minimal · up to dab72

This localized change switches terminal plot sizing to the canonical API and verifies the expected 80 by 30 dimensions; no actionable merge-blocking risk remains after normal checks and review.

Poem

Pareto bars stand neat and bright,
Sized by plotsize, just right.
ANSI, plain, both tests agree,
Eighty wide by thirty free.
Old API drifts out of sight.

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: using the canonical Plotext plotsize API.
Description check ✅ Passed The description includes the required overview, details, reviewer-start files, related issue, and validation information.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

Comment @coderabbitai help to get the list of available commands.

@natoscott

natoscott commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Hi — heads up that this will still fail in CI. The pre-built venv appears to have a version of plotext with no plotsize either. PR #1589 adds an early-exit guard plus the size-function probe.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants