chore: address Andre's feedback on --save-results and CLI preview by johnnygreco · Pull Request #335 · NVIDIA-NeMo/DataDesigner

johnnygreco · 2026-02-18T23:01:34Z

📋 Summary

Addresses Andre's feedback on the --save-results CLI feature and preview UX. Key changes include making --save-results skip terminal display (write-only), suppressing spurious stdout when saving to file, capping terminal display width, threading --theme through to the pager shell, adding wrap-around navigation, and removing the allow_resize feature from the batch pipeline.

🔄 Changes

✨ Added

Light theme CSS variables for the sample records browser pager — --theme now applies to the pager shell, not just individual record HTML files
Wrap-around navigation in both the terminal interactive browser and HTML pager (last→first, first→last)
Tests for dark/light theme support in the pager (test_sample_records_pager.py)

🔧 Changed

--save-results now writes artifacts to disk without displaying sample records in the terminal (previously did both)
Terminal display width is capped at --display-width via min(terminal_width, display_width) instead of using display_width unconditionally
display_sample_record and generate_analysis_report use io.StringIO() as the console file when recording, preventing spurious stdout
--display-width and --theme help text updated to be clearer
DatasetBatchManager.replace_buffer split into update_records (strict 1:1) and replace_buffer (allows resize for processors)
Refactored _save_preview_results into its own method on GenerationController
Reused record_series fixture across visualization tests

🗑️ Removed

allow_resize feature — removed allow_resize field from SingleColumnConfig, all resize logic from CustomColumnGenerator, ColumnWiseDatasetBuilder, and DatasetBatchManager (~590 lines removed)
example_allow_resize.py demo script
Resize documentation from custom_columns.md and example.md
All resize-related tests (test_custom.py, test_column_wise_builder.py, test_dataset_batch_manager.py)
Verbose CLI docstring examples from preview command (kept single-line summary)

🔍 Attention Areas

⚠️ Reviewers: Please pay special attention to the following:

generation_controller.py — Core logic change: --save-results now skips terminal display entirely; the save vs display paths are now mutually exclusive
dataset_batch_manager.py — replace_buffer / update_records split — verify the processor pipeline still uses replace_buffer correctly
column_wise_builder.py — Significant simplification from removing all resize bookkeeping

🤖 Generated with AI

Console(record=True) still prints to stdout by default. Use file=io.StringIO() to redirect output so save-path calls only write to disk.

When --save-results is used, records and the analysis report are no longer printed to the terminal. Extracted save logic into a dedicated _save_preview_results method and updated option help text accordingly.

Prev/next buttons and arrow keys now cycle back to the beginning/end instead of clamping at boundaries.

The pager shell was hardcoded dark, so --theme light produced light records inside a dark frame. Extract CSS variables into dark/light constants and pass the theme from the controller.

The module-level Console() had no width limit, so tables with expand=True stretched to the full terminal width. Cap terminal output at min(terminal_width, display_width) and thread the display_width parameter through the controller's display methods.

Remove "Only applies when --save-results is used" from --display-width since it now also affects terminal output.

greptile-apps · 2026-02-18T23:04:01Z

Greptile Summary

This PR improves the --save-results CLI feature by making it write-only (no terminal display), suppressing spurious stdout when saving files, capping terminal display width, and threading the --theme parameter through to the HTML pager shell. The changes also add wrap-around navigation for both the terminal interactive browser and HTML pager.

Key Changes

--save-results now skips terminal display entirely — the save and display paths are mutually exclusive in generation_controller.py:73-85
Spurious stdout suppressed by using io.StringIO() as the console file parameter in reporting.py:149 and visualization.py:439
Terminal display width capped at min(terminal_width, display_width) in visualization.py:443-445
--theme parameter now applies to the pager shell HTML, not just individual record files — light theme CSS variables added in sample_records_pager.py:258-270
Wrap-around navigation implemented using modulo arithmetic in both terminal browser and HTML pager
Help text updated to clarify behavior — verbose examples removed from preview command docstring
Tests updated to match new behavior, with record_series fixture reused across visualization tests

Confidence Score: 5/5

This PR is safe to merge with minimal risk
All changes are well-contained refactoring improvements with comprehensive test coverage. The mutually exclusive save vs display paths are clean, the io.StringIO() fix properly suppresses stdout, and the width capping logic is correct. Wrap-around navigation using modulo arithmetic is a standard pattern. Tests have been updated to match all behavioral changes.
No files require special attention

Important Files Changed

Filename	Overview
packages/data-designer/src/data_designer/cli/controllers/generation_controller.py	Refactored `--save-results` to skip terminal display, extracted save logic into `_save_preview_results`, and threaded `display_width` through all display methods
packages/data-designer-config/src/data_designer/config/utils/visualization.py	Prevented spurious stdout when saving by using `io.StringIO()`, and capped terminal display width at `min(terminal_width, display_width)`
packages/data-designer-config/src/data_designer/config/analysis/utils/reporting.py	Fixed spurious stdout by using `io.StringIO()` when recording console output for file saves
packages/data-designer/src/data_designer/cli/utils/sample_records_pager.py	Added light theme support and wrap-around navigation (modulo arithmetic) for the HTML pager browser

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[run_preview called] --> B{save_results flag?}
    B -->|True| C[_save_preview_results]
    B -->|False| D{interactive mode?}
    
    C --> C1[Create timestamped directory]
    C1 --> C2[Save analysis report HTML]
    C2 --> C3[Save dataset parquet]
    C3 --> C4[Loop: Save each record HTML]
    C4 --> C5[create_sample_records_pager with theme]
    C5 --> C6[Log paths only, no terminal display]
    
    D -->|Yes| E[_browse_records_interactively]
    D -->|No| F[_display_all_records]
    
    E --> E1[Display record with header]
    E1 --> E2[wait_for_navigation_key]
    E2 -->|n/enter| E3[index = index + 1 mod total]
    E2 -->|p| E4[index = index - 1 + total mod total]
    E2 -->|q| E5[Exit loop]
    E3 --> E1
    E4 --> E1
    E5 --> G[Display analysis to console]
    
    F --> F1[Loop through all records]
    F1 --> F2[_display_record_with_header]
    F2 --> G
    
    C6 --> H[Complete]
    G --> H
    
    I[display_sample_record] --> J{save_path provided?}
    J -->|Yes| K[Create Console with io.StringIO]
    J -->|No| L[Create Console with capped width]
    K --> M[Record output]
    L --> N[Display to terminal]
    M --> O[Save as HTML/SVG with theme]
    
    style C fill:#e1f5ff
    style E fill:#fff4e1
    style F fill:#fff4e1
    style K fill:#e8f5e9
    style L fill:#e8f5e9

_{Last reviewed commit: d9232f5}

…ve_results behavior

johnnygreco · 2026-02-18T23:34:48Z

packages/data-designer-config/src/data_designer/config/analysis/utils/reporting.py

    if save_path is not None:
+        recording_console = Console(record=True, file=io.StringIO())
+        recording_console.print(Group(*render_list), markup=False)
        save_path = str(save_path)


don't print to the terminal when --save-results is passed

johnnygreco · 2026-02-18T23:35:24Z

packages/data-designer/src/data_designer/cli/commands/preview.py

-    """Generate a preview dataset for fast iteration on your configuration.
-
-    Preview results are displayed in the terminal. Use this to quickly validate
-    your configuration before running a full dataset creation.
-
-    By default, records are displayed one at a time in interactive mode. Use
-    --non-interactive to display all records at once (also used automatically


didn't realize the entire docstring prints on --help

johnnygreco · 2026-02-18T23:35:52Z

packages/data-designer/src/data_designer/cli/utils/sample_records_pager.py

+_LIGHT_CSS_VARS = """\
+  color-scheme: light;
+  --panel: rgba(255, 255, 255, 0.92);
+  --border: rgba(0, 0, 0, 0.12);
+  --text: #1a1a2e;
+  --muted: #5a6078;
+  --shadow: 0 16px 44px rgba(0, 0, 0, 0.08);
+  --body-bg: linear-gradient(180deg, #f0f2f8 0%, #e8eaf0 45%, #f5f6fa 100%);
+  --topbar-bg: linear-gradient(135deg, rgba(255,255,255,0.95) 0%, rgba(248,249,252,0.98) 100%);
+  --btn-bg: linear-gradient(180deg, rgba(255,255,255,0.9) 0%, rgba(240,242,248,0.95) 100%);
+  --btn-hover-border: rgba(59, 130, 246, 0.6);
+  --btn-hover-glow: 0 0 0 3px rgba(59, 130, 246, 0.12);
+  --frame-bg: rgba(255, 255, 255, 0.95);"""
+
+


makes the full browser light

andreatgretel

thanks, looks pretty good!!

johnnygreco requested a review from a team as a code owner February 18, 2026 23:01

johnnygreco added 7 commits February 18, 2026 18:03

fix: suppress stdout when saving report and sample records to file

796acfe

Console(record=True) still prints to stdout by default. Use file=io.StringIO() to redirect output so save-path calls only write to disk.

refactor: --save-results skips terminal display

1bd97f6

When --save-results is used, records and the analysis report are no longer printed to the terminal. Extracted save logic into a dedicated _save_preview_results method and updated option help text accordingly.

feat: wrap-around navigation in sample records browser

2c481ef

Prev/next buttons and arrow keys now cycle back to the beginning/end instead of clamping at boundaries.

test: reuse record_series fixture in visualization tests

2c88b04

feat: thread --theme through to sample records pager

a37e972

The pager shell was hardcoded dark, so --theme light produced light records inside a dark frame. Extract CSS variables into dark/light constants and pass the theme from the controller.

docs: update --display-width and --theme help text

851c7f0

Remove "Only applies when --save-results is used" from --display-width since it now also affects terminal output.

fix: update generation controller tests to match display_width and sa…

d9232f5

…ve_results behavior

johnnygreco requested a review from andreatgretel February 18, 2026 23:34

johnnygreco commented Feb 18, 2026

View reviewed changes

andreatgretel approved these changes Feb 19, 2026

View reviewed changes

johnnygreco merged commit 03b3d6c into main Feb 19, 2026
47 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comments

chore: address Andre's feedback on --save-results and CLI preview#335

chore: address Andre's feedback on --save-results and CLI preview#335
johnnygreco merged 8 commits intomainfrom
johnny/chore/updates-to-save-results-from-andre

johnnygreco commented Feb 18, 2026

Uh oh!

greptile-apps bot commented Feb 18, 2026 •

edited

Loading

Confidence Score: 5/5

Flowchart

Uh oh!

johnnygreco Feb 18, 2026

Uh oh!

johnnygreco Feb 18, 2026

Uh oh!

johnnygreco Feb 18, 2026

Uh oh!

andreatgretel left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Comments

Conversation

johnnygreco commented Feb 18, 2026

📋 Summary

🔄 Changes

✨ Added

🔧 Changed

🗑️ Removed

🔍 Attention Areas

Uh oh!

greptile-apps bot commented Feb 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Key Changes

Confidence Score: 5/5

Important Files Changed

Flowchart

Uh oh!

johnnygreco Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

johnnygreco Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

johnnygreco Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

andreatgretel left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

greptile-apps bot commented Feb 18, 2026 •

edited

Loading