Skip to content

Conversation

@nabinchha
Copy link
Contributor

@nabinchha nabinchha commented Feb 10, 2026

📋 Summary

Consolidate all storage logic under engine/storage/ by moving ArtifactStorage alongside MediaStorage for better code organization.

🔄 Changes

🔧 Changed

  • Moved artifact_storage.py from engine/dataset_builders/ to engine/storage/
  • Moved test_artifact_storage.py from tests/engine/dataset_builders/ to tests/engine/storage/
  • Updated engine/storage/__init__.py to export ArtifactStorage, BatchStage, and related constants
  • Updated imports across 18 source and test files

#317 needs to merge first

…eInferenceParameters, EmbeddingInferenceParameters
…lved based on the type of InferenceParameters
Consolidate all storage logic (ArtifactStorage, MediaStorage) under
engine/storage/ for better organization. Update all imports across
source and test files.
@greptile-apps
Copy link

greptile-apps bot commented Feb 10, 2026

Greptile Overview

Greptile Summary

This PR consolidates storage-related code by moving ArtifactStorage from engine/dataset_builders/ to engine/storage/ alongside MediaStorage. All 18 import statements across source and test files have been updated to reference the new location.

Key changes:

  • Moved artifact_storage.py to engine/storage/ module with no content changes
  • Moved test_artifact_storage.py to tests/engine/storage/ with updated mock patch path
  • Updated imports in 18 files from dataset_builders.artifact_storage to storage.artifact_storage
  • Removed duplicate _has_image_columns method in column_wise_builder.py

Note: The PR description mentions updating engine/storage/__init__.py to export ArtifactStorage, BatchStage, and related constants, but this file was not modified in the changeset. The __init__.py currently only exports MediaStorage and StorageMode.

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk
  • This is a straightforward refactoring that moves files and updates import paths without changing any logic. All imports have been systematically updated, including TYPE_CHECKING blocks and mock patch paths. The removal of the duplicate method is a minor cleanup improvement.
  • No files require special attention

Important Files Changed

Filename Overview
packages/data-designer-engine/src/data_designer/engine/storage/artifact_storage.py File moved from dataset_builders/ to storage/ without content changes
packages/data-designer-engine/tests/engine/storage/test_artifact_storage.py Test file moved to match new location, imports updated correctly, patch path updated
packages/data-designer-engine/src/data_designer/engine/dataset_builders/column_wise_builder.py Import updated and duplicate _has_image_columns method removed
packages/data-designer/src/data_designer/integrations/huggingface/client.py Import path updated to new storage location for constants
packages/data-designer/src/data_designer/interface/data_designer.py Import path updated to new storage location

Last reviewed commit: f0ea307

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

20 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

Comment on lines 23 to 25
"MediaStorage",
"StorageMode",
]
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing trailing newline

packages/data-designer-engine/src/data_designer/engine/storage/__init__.py appears to be missing a final newline at EOF. Some tooling (e.g., strict linters / formatting checks) treats this as a failure. Adding a newline after the closing ] avoids CI/style check issues.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/data-designer-engine/src/data_designer/engine/storage/__init__.py
Line: 23:25

Comment:
**Missing trailing newline**

`packages/data-designer-engine/src/data_designer/engine/storage/__init__.py` appears to be missing a final newline at EOF. Some tooling (e.g., strict linters / formatting checks) treats this as a failure. Adding a newline after the closing `]` avoids CI/style check issues.

<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>

How can I resolve this? If you propose a fix, please make it concise.

johnnygreco
johnnygreco previously approved these changes Feb 12, 2026
Base automatically changed from nmulepati/feat/125-support-image-generation to main February 12, 2026 21:00
@nabinchha nabinchha dismissed johnnygreco’s stale review February 12, 2026 21:00

The base branch was changed.

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

20 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

@greptile-apps
Copy link

greptile-apps bot commented Feb 12, 2026

Additional Comments (1)

packages/data-designer-engine/src/data_designer/engine/storage/__init__.py
PR description states this file should export ArtifactStorage, BatchStage, and related constants, but they're not in __all__. Consider adding them for consistency with the PR description.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/data-designer-engine/src/data_designer/engine/storage/__init__.py
Line: 6:6

Comment:
PR description states this file should export `ArtifactStorage`, `BatchStage`, and related constants, but they're not in `__all__`. Consider adding them for consistency with the PR description.

<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>

How can I resolve this? If you propose a fix, please make it concise.

@nabinchha nabinchha merged commit 1f172ea into main Feb 12, 2026
47 checks passed
@nabinchha nabinchha deleted the nmulepati/chore/move-artifact-storage branch February 12, 2026 21:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants