feat: add --save-results option to preview command by johnnygreco · Pull Request #333 · NVIDIA-NeMo/DataDesigner

johnnygreco · 2026-02-18T19:34:22Z

📋 Summary

Add --save-results flag to the data-designer preview CLI command, enabling users to persist preview artifacts (dataset parquet, analysis report HTML, and per-record sample HTML files) to disk with a built-in browser pager for navigating sample records.

🔄 Changes

✨ Added

sample_records_pager.py — Generates a single-page HTML pager (sample_records_browser.html) for navigating saved sample records via iframe with prev/next, dropdown jump, and keyboard navigation
--save-results, --artifact-path, --theme, and --display-width CLI options on preview command
save_path, theme, and display_width parameters on display_sample_record() and WithRecordSamplerMixin.display_sample_record()
HTML post-processing pipeline: viewport meta injection and dark-mode CSS for saved record files
Comprehensive test coverage for all new functionality

🔧 Changed

generation_controller.py — run_preview() now orchestrates saving dataset parquet, analysis report, sample record HTMLs, and the pager when --save-results is active
visualization.py — display_sample_record() supports saving to HTML/SVG via Rich's recording console
Collapsed redundant preview command delegation tests into a single parametrized test
Added missing type annotations to convert_to_row_element and pad_console_element

🔍 Attention Areas

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

visualization.py — HTML post-processing uses regex to inject CSS before </head>. Uses a lambda replacement to avoid backslash interpretation issues.
generation_controller.py — Save-results block creates a timestamped directory structure. Catches OSError specifically (not broad Exception).
sample_records_pager.py — New file (~270 lines) generating a self-contained HTML pager via f-string template.

🤖 Generated with AI

greptile-apps · 2026-02-18T19:38:15Z

Greptile Summary

This PR adds a --save-results flag to the data-designer preview CLI command, enabling users to persist preview artifacts to disk. When activated, it creates a timestamped directory containing the dataset as Parquet, an analysis report as HTML, per-record sample HTML files, and a self-contained HTML pager for navigating records in a browser.

Adds --save-results, --artifact-path, --theme, and --display-width CLI options to the preview command
Introduces apply_html_post_processing() to inject viewport meta and dark-mode CSS into Rich-exported HTML files
Creates sample_records_pager.py generating a polished single-page HTML browser with iframe navigation, keyboard shortcuts, and dropdown record selection
Fixes a latent UnboundLocalError bug in WithRecordSamplerMixin.display_sample_record() where num_records was defined inside the try block but referenced in the except handler
Comprehensive test coverage across all new functionality with well-structured parametrized tests

Confidence Score: 4/5

This PR is safe to merge — it adds a well-tested opt-in feature with no changes to existing behavior unless --save-results is used.
Score of 4 reflects that the feature is well-implemented with comprehensive tests, proper error handling, and good separation of concerns. The only concern is a minor UX issue where --display-width silently does nothing without --save-results. The bug fix for the UnboundLocalError in the mixin is a genuine improvement.
packages/data-designer-config/src/data_designer/config/utils/visualization.py — core save logic with HTML post-processing regex; packages/data-designer/src/data_designer/cli/controllers/generation_controller.py — save orchestration with OSError handling

Important Files Changed

Filename	Overview
packages/data-designer-config/src/data_designer/config/utils/constants.py	Adds `DEFAULT_DISPLAY_WIDTH = 110` constant. Clean, minimal change.
packages/data-designer-config/src/data_designer/config/utils/visualization.py	Adds save-to-file support (`save_path`, `theme`, `display_width` params), HTML post-processing with viewport/dark-mode injection, and fixes an `UnboundLocalError` bug by moving `num_records` before the `try` block. Well-structured with good separation of concerns.
packages/data-designer-config/tests/config/utils/test_visualization.py	Comprehensive test coverage for new save functionality, HTML post-processing, SVG output, invalid extensions, and the `UnboundLocalError` bug fix. Has known test naming issue (`testapply_` instead of `test_apply_`) noted in prior review.
packages/data-designer/src/data_designer/cli/commands/preview.py	Adds `--save-results`, `--artifact-path`, `--theme`, and `--display-width` CLI options. Clean delegation to `GenerationController.run_preview()`.
packages/data-designer/src/data_designer/cli/controllers/generation_controller.py	Orchestrates save-results workflow: timestamped directory creation, dataset parquet export, report HTML, per-record HTML save, and pager generation. `OSError` handling with `typer.Exit(code=1)`. One potential issue with `--display-width` help text being ambiguous about scope.
packages/data-designer/src/data_designer/cli/utils/sample_records_pager.py	New file generating a self-contained HTML pager for browsing sample records via iframe. Properly escapes directory names with `html.escape()`, uses `json.dumps()` for record file list, and includes keyboard/button navigation. Well-structured dark-themed UI.
packages/data-designer/tests/cli/commands/test_preview_command.py	Collapsed three similar tests into a single parametrized test covering all new CLI options. Clean improvement.
packages/data-designer/tests/cli/controllers/test_generation_controller.py	Thorough tests for save-results workflow: directory structure, default artifact path, analysis presence/absence, OSError handling, and non-OSError propagation. Well-structured mock setup.
packages/data-designer/tests/cli/utils/test_sample_records_pager.py	Good coverage for pager generation: zero/single/multiple records, XSS-safe title escaping, valid JSON records array, and counter element presence.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["preview_command()"] --> B["GenerationController.run_preview()"]
    B --> C["Generate preview dataset"]
    C --> D{Interactive?}
    D -->|Yes| E["_browse_records_interactively()"]
    D -->|No| F["_display_all_records()"]
    E --> G["display_sample_record(index=i)"]
    F --> G
    G --> H{analysis present?}
    H -->|Yes| I["analysis.to_report() → console"]
    H -->|No| J{--save-results?}
    I --> J
    J -->|No| K["Print success"]
    J -->|Yes| L["Create timestamped directory"]
    L --> M["Save report.html"]
    L --> N["Save dataset.parquet"]
    L --> O["Save record_i.html per record"]
    O --> P["apply_html_post_processing()"]
    P --> Q["create_sample_records_pager()"]
    Q --> R["Write sample_records_browser.html"]
    R --> K

_{Last reviewed commit: 52245b7}

greptile-apps

_{8 files reviewed, 2 comments}

_{Edit Code Review Agent Settings | Greptile}

packages/data-designer-config/tests/config/utils/test_visualization.py

packages/data-designer/src/data_designer/cli/controllers/generation_controller.py

Allow saving rendered sample records as HTML or SVG files via an optional save_path parameter on both the standalone function and the WithRecordSamplerMixin method.

Replace the single-file --save-report option with --save-results, which saves all preview artifacts (dataset parquet, analysis report HTML, and per-record sample HTMLs) into a timestamped directory under the artifact path. Add error handling around the save block, improve timestamp precision to microseconds, and expand test coverage for the new behavior.

… and UI polish

- Split try/except in generation_controller so report display errors don't produce misleading "failed to save" messages when not saving - Add browser HTML path to save success output for discoverability - Remove 5 unused CSS variables from pager theme constants - Add "N of M" record counter to pager toolbar - Add theme/display_width assertions to all preview_command tests - Add dedicated test for custom theme and display_width passthrough - Add tests for record counter and CSS variable cleanup

- Fix critical bug: analysis report now displays to console even when --save-results is active (was silently dropped via pass statement) - Fix latent UnboundLocalError in display_sample_record when index is out of bounds (num_records computed before try block) - Eliminate duplicated dark CSS between constant and theme listener script - Simplify sample_records_pager: remove dual-theme system, postMessage bridge, and responsive media queries; restore GitHub link; reorder toolbar to put prev/next buttons on the far left - Narrow except Exception to except OSError in save-results path - Use case-insensitive extension check and lambda-based re.sub - Collapse redundant preview command delegation tests into parametrize - Add missing type annotations and remove tautological assertions

_THEME_LISTENER_SCRIPT and _SAMPLE_RECORD_DARK_CSS_INLINE became orphaned after the pager simplification removed the postMessage bridge. This removes both constants, drops the injection line, switches the idempotency guard to the viewport meta tag, and cleans up related test assertions.

greptile-apps

_{8 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

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

greptile-apps

_{8 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

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

packages/data-designer-config/tests/config/utils/test_visualization.py

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

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

…post_processing public

greptile-apps

_{9 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

packages/data-designer-config/tests/config/utils/test_visualization.py

…ation.py Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>

nabinchha

🚢

greptile-apps

_{9 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps · 2026-02-18T20:55:42Z

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

+    display_width: int = typer.Option(
+        DEFAULT_DISPLAY_WIDTH,
+        "--display-width",
+        help="Width of the rendered record output in characters.",
+        min=40,
+    ),


--display-width silently ignored without --save-results

The --display-width option is accepted regardless of whether --save-results is passed, but it has no effect on the console display path. In visualization.py, when save_path is None, the module-level console object is used with its own default width, and display_width is ignored.

A user running data-designer preview config.yaml --display-width 80 (without --save-results) would expect narrower output but get no change.

Consider either:

Applying display_width to the console output path as well, or

Adding a note to the help text that this only applies with --save-results:

Suggested change

display_width: int = typer.Option(

DEFAULT_DISPLAY_WIDTH,

"--display-width",

help="Width of the rendered record output in characters.",

min=40,

),

display_width: int = typer.Option(

DEFAULT_DISPLAY_WIDTH,

"--display-width",

help="Width of the rendered record output in characters. Only applies when --save-results is used.",

min=40,

),

Prompt To Fix With AI

This is a comment left during a code review. Path: packages/data-designer/src/data_designer/cli/commands/preview.py Line: 49-54 Comment: **`--display-width` silently ignored without `--save-results`** The `--display-width` option is accepted regardless of whether `--save-results` is passed, but it has no effect on the console display path. In `visualization.py`, when `save_path is None`, the module-level `console` object is used with its own default width, and `display_width` is ignored. A user running `data-designer preview config.yaml --display-width 80` (without `--save-results`) would expect narrower output but get no change. Consider either: 1. Applying `display_width` to the console output path as well, or 2. Adding a note to the help text that this only applies with `--save-results`: ```suggestion display_width: int = typer.Option( DEFAULT_DISPLAY_WIDTH, "--display-width", help="Width of the rendered record output in characters. Only applies when --save-results is used.", min=40, ), ``` How can I resolve this? If you propose a fix, please make it concise.

andreatgretel · 2026-02-18T21:10:23Z

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

+<title>{title}</title>
+<style>
+:root {{
+  color-scheme: dark;


the pager shell is hardcoded dark, so if someone passes --theme light they get white-background records sitting inside the dark pager frame — looks a bit odd. might be worth threading the theme through to _build_pager_html too, or at least a note in the help text

nice catch! that was intentional, but now that you call it out, i think you are right 🤦‍♂️

andreatgretel · 2026-02-18T21:10:23Z

packages/data-designer-config/src/data_designer/config/utils/visualization.py

@@ -622,3 +647,53 @@ def _get_field_constraints(field: dict, schema: dict) -> str:
            constraints.append(f"allowed: {', '.join(enum_values.keys())}")



this flips the background to dark blue but Rich's default markup colors (greens, reds, dim text) were designed for white. they're not broken, but some of them feel a bit washed out on the dark bg. the dracula code blocks look great since that theme was made for dark — just something to keep an eye on if you want to polish the dark mode later

Good callout! For sure would be a good idea to iterate once more folks have seen it on their screen

andreatgretel · 2026-02-18T21:19:42Z

packages/data-designer/src/data_designer/cli/controllers/generation_controller.py

+                results.dataset.to_parquet(results_dir / "dataset.parquet")
+
+                sample_records_dir = results_dir / "sample_records"
+                sample_records_dir.mkdir(parents=True, exist_ok=True)


looks like when --save-results is used, records and the report might get printed to the terminal twice? the display happens above (lines 69-76), and then this save block calls display_sample_record and to_report again with save_path — but those internally use Console(record=True) which still writes to stdout on top of saving to file. haven't tested it end-to-end with the full pipeline so not 100% sure, but might be worth a quick check

andreatgretel · 2026-02-18T21:21:14Z

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

+        help="Width of the rendered record output in characters.",
+        min=40,
+    ),
 ) -> None:


minor: --theme help text says it only applies with --save-results, but --display-width doesn't mention that — it also only affects saved files (console display uses terminal width). might be worth adding the same note for consistency, or maybe a shared "Save Options" section in the help output if typer supports that

andreatgretel · 2026-02-18T21:44:28Z

packages/data-designer-config/tests/config/utils/test_visualization.py

+    """Test that display_sample_record saves HTML with dark-mode style block injected."""
+    sample_record = {"code": "print('hello world')", "code_validation_result": validation_output}
+    record_series = pd.Series(sample_record)
+    save_path = tmp_path / "output.html"


nit: the sample_record + pd.Series setup is repeated in 5 tests (save_html, save_svg, save_invalid_extension, save_path_none_default, svg_no_dark_mode). could be a fixture:

@pytest.fixture def record_series(validation_output: dict) -> pd.Series: return pd.Series({"code": "print('hello world')", "code_validation_result": validation_output})

johnnygreco requested a review from a team as a code owner February 18, 2026 19:34

greptile-apps bot reviewed Feb 18, 2026

View reviewed changes

packages/data-designer-config/tests/config/utils/test_visualization.py Outdated Show resolved Hide resolved

packages/data-designer/src/data_designer/cli/controllers/generation_controller.py Show resolved Hide resolved

johnnygreco added 9 commits February 18, 2026 14:41

feat: add --save-report option to preview command

3c9d7cc

feat: add save_path option to display_sample_record

ed560ab

Allow saving rendered sample records as HTML or SVG files via an optional save_path parameter on both the standalone function and the WithRecordSamplerMixin method.

feat: add sample records pager with theme toggle, postMessage bridge,…

dba0e8c

… and UI polish

feat: add dataset metadata subtitle to pager and clean up toolbar layout

5a7b9da

style: move record counter to far right of pager toolbar

e6fef53

johnnygreco force-pushed the johnny/feat/preview-save-report branch from 67c8ef6 to ad4c9b2 Compare February 18, 2026 19:41

greptile-apps bot reviewed Feb 18, 2026

View reviewed changes

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

johnnygreco added 3 commits February 18, 2026 14:55

fix: move Path import out of TYPE_CHECKING block in test_visualization

4f4bcd4

fix: rename _logger to logger to match codebase convention

e392f2e

fix: remove unnecessary cast in preview command theme parameter

4e02fd8

greptile-apps bot reviewed Feb 18, 2026

View reviewed changes

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

nabinchha previously approved these changes Feb 18, 2026

View reviewed changes

refactor: extract DEFAULT_DISPLAY_WIDTH constant and make apply_html_…

a32155c

…post_processing public

johnnygreco dismissed nabinchha’s stale review via a32155c February 18, 2026 20:45

greptile-apps bot reviewed Feb 18, 2026

View reviewed changes

packages/data-designer-config/tests/config/utils/test_visualization.py Outdated Show resolved Hide resolved

Update packages/data-designer-config/tests/config/utils/test_visualiz…

52245b7

…ation.py Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>

nabinchha approved these changes Feb 18, 2026

View reviewed changes

greptile-apps bot reviewed Feb 18, 2026

View reviewed changes

johnnygreco merged commit f2a1657 into main Feb 18, 2026
47 checks passed

andreatgretel reviewed Feb 18, 2026

View reviewed changes

		@@ -622,3 +647,53 @@ def _get_field_constraints(field: dict, schema: dict) -> str:
		constraints.append(f"allowed: {', '.join(enum_values.keys())}")

Conversation

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

Uh oh!

📋 Summary

🔄 Changes

✨ Added

🔧 Changed

🔍 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

Confidence Score: 4/5

Important Files Changed

Flowchart

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nabinchha left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

andreatgretel 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 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 Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

andreatgretel Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

andreatgretel Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Comments

johnnygreco commented Feb 18, 2026 •

edited

Loading

greptile-apps bot commented Feb 18, 2026 •

edited

Loading