feat: new parameter types and handlers: Directory and FileSequence#4635
feat: new parameter types and handlers: Directory and FileSequence#4635alex-rumsey-foundry wants to merge 31 commits into
Conversation
| def image_sequence_destination(self) -> ImageSequenceDestination | None: ... | ||
|
|
||
|
|
||
| class ProjectImageSequenceParameter: |
There was a problem hiding this comment.
is this unique to images? Could I use it for dialogue files, too? After convos with Jason we'd like to ensure that other sequential artifact types can benefit from it, so I'll be moving away from terms like "frames".
| macro_path: MacroPath = self._dir_path # type: ignore[assignment] | ||
|
|
||
| for index in range(1, _MAX_VERSION_INDEX + 1): | ||
| variables = {**macro_path.variables, "_index": index} |
There was a problem hiding this comment.
I'll have to double check on this, but I don't think there's a hardcoded use of index as the indexing variable. Also note that the _index (underbar) is a prepend to put an underbar when rendered.
| def _create_direct(self) -> Directory: | ||
| """Create the directory without versioning.""" | ||
| resolved = Path(_resolve_dir_path(self._dir_path)) | ||
|
|
||
| if resolved.exists() and self._existing_dir_policy == ExistingFilePolicy.FAIL: | ||
| msg = f"Attempted to create directory. Failed because directory already exists: {resolved}" | ||
| raise DirectoryError(msg) | ||
|
|
||
| resolved.mkdir(parents=self._create_parents, exist_ok=True) | ||
| return _map_to_macro_directory(resolved, self._dir_path) |
There was a problem hiding this comment.
if we have existing request to cover this, it'd be great to use it
There was a problem hiding this comment.
Added a MakeDirectoryRequest
There was a problem hiding this comment.
Pull request overview
This PR adds first-class filesystem output shapes beyond a single file path by introducing Directory and ImageSequence types, along with project-aware parameter components and new default project situations to generate versioned output directories and frame sequences.
Changes:
- Added
Directory/DirectoryDestinationandImageSequence/ImageSequenceDestinationfilesystem abstractions. - Added
ProjectDirectoryParameterandProjectImageSequenceParameterparam components to build destinations from project situations (including “cog” settings-node wiring). - Extended the default project template with
save_output_directoryandsave_image_sequence_framesituations.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/griptape_nodes/files/image_sequence.py | Introduces ImageSequence read descriptor, ImageSequenceDestination write handle, and frame-pattern conversion helpers. |
| src/griptape_nodes/files/directory.py | Introduces Directory path wrapper and DirectoryDestination for creation/versioning + mapping back to project macros. |
| src/griptape_nodes/files/init.py | Exposes the new Directory/ImageSequence APIs from the files package. |
| src/griptape_nodes/exe_types/param_components/project_image_sequence_parameter.py | Adds a project-aware parameter component that builds image-sequence destinations from situations and supports settings-node wiring. |
| src/griptape_nodes/exe_types/param_components/project_directory_parameter.py | Adds a project-aware parameter component that builds directory destinations from situations and supports settings-node wiring. |
| src/griptape_nodes/common/project_templates/default_project_template.py | Adds two new default situations for versioned directory and image-sequence outputs. |
Comments suppressed due to low confidence (3)
src/griptape_nodes/files/image_sequence.py:214
build_versioned_sequence_destinationalways scans for a non-existent parent directory and locks a new_indexeven whenexisting_file_policyis OVERWRITE or FAIL. That means callers cannot opt into reusing an existing versioned directory via situation policy (OVERWRITE/FAIL) or by supplying a specific_index. Consider only performing the version scan when the caller intends CREATE_NEW behavior (or add an explicit versioning policy parameter), and otherwise lock_indexdeterministically (e.g., 1 or a caller-supplied value).
def build_versioned_sequence_destination(
frame_macro: MacroPath,
*,
existing_file_policy: ExistingFilePolicy = ExistingFilePolicy.OVERWRITE,
create_parents: bool = True,
) -> ImageSequenceDestination:
"""Find the first available version index and return a locked ImageSequenceDestination.
Increments ``_index`` in the frame macro starting at 1 until the corresponding
parent directory does not exist. Returns an ``ImageSequenceDestination`` with
that index locked into the variables dict.
src/griptape_nodes/files/directory.py:231
- When mapping the created directory back to a portable project macro fails,
_map_to_macro_directorycurrently returnsDirectory(fallback_path)iffallback_pathis aMacroPath. That meansDirectory.locationwill be the unresolved template (dropping the locked_indexvariables), which is easy for callers to accidentally persist and later fail to resolve. Consider falling back toDirectory(str(absolute_path))(mirroringProjectFileDestination’s behavior of returning an absolute-path File when mapping fails), or provide a way to serialize both template + variables.
map_result = GriptapeNodes.handle_request(AttemptMapAbsolutePathToProjectRequest(absolute_path=absolute_path))
if isinstance(map_result, AttemptMapAbsolutePathToProjectResultSuccess) and map_result.mapped_path is not None:
return Directory(map_result.mapped_path)
if isinstance(fallback_path, MacroPath):
return Directory(fallback_path)
return Directory(str(absolute_path))
src/griptape_nodes/files/directory.py:154
DirectoryDestination.create()routes anyCREATE_NEW+MacroPathinto_create_with_versioning(), but it doesn’t verify that the macro template actually references{_index}. If the directory already exists and the template doesn’t use_index, the loop will repeatedly resolve the same path and eventually raise the misleading “no available path found after 9999 attempts”. Consider checking whether_indexis referenced inmacro_path.parsed_macrobefore versioning, and either fall back to direct creation (with an appropriate policy) or raise a targeted error explaining that CREATE_NEW requires an{_index}slot.
def create(self) -> Directory:
"""Create the directory and return a Directory referencing it.
When policy is CREATE_NEW and the path is a MacroPath containing
``{_index}``, increments the index starting at 1 until a non-existent
directory is found, then creates it.
Returns:
Directory referencing the created path (in macro form if inside project).
Raises:
DirectoryError: If the directory cannot be created.
"""
if self._existing_dir_policy == ExistingFilePolicy.CREATE_NEW and isinstance(self._dir_path, MacroPath):
return self._create_with_versioning()
return self._create_direct()
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def _on_frame_written(self, written_file: File) -> None: # noqa: ARG002 | ||
| """Record that a frame was written to expose the ImageSequence descriptor.""" | ||
| if self._written_sequence is None: | ||
| self._written_sequence = ImageSequence(self._frame_macro) | ||
|
|
| def __init__( | ||
| self, | ||
| dir_path: str | MacroPath, | ||
| *, | ||
| existing_dir_policy: ExistingFilePolicy = ExistingFilePolicy.CREATE_NEW, | ||
| create_parents: bool = True, | ||
| ) -> None: | ||
| """Store directory path and creation configuration. No I/O is performed. |
* fix: register libraries inside generated build_workflow() Closes #4584 * test: add cold-runtime and e2e coverage for #4584 - Unit test exec()s a generated workflow under a recording stub for ahandle_request and asserts every RegisterLibraryFromFileRequest precedes every CreateNodeRequest in build_workflow(). - New tests/e2e tier spawns a fresh python subprocess against a generator- produced workflow that uses an Echo Library fixture, verifying the standalone path produces real nodes (not ErrorProxyNode) end-to-end. - Makefile gains `make test/e2e`; `make test` now runs unit + integration + e2e. * ci: add e2e test job and rename unit-tests workflow to tests Pulls e2e into the same triggers (push to main, PR to main, merge_group) on ubuntu-latest as a separate job. Workflow file renamed from unit-tests.yml to tests.yml and the displayed name changed to 'Tests' since it now covers more than just unit. * ci: also run e2e tests on windows
) Adds `existing_path` to `UpdateLibraryResultFailure` and `DownloadLibraryResultFailure` and populates it on the uncommitted-changes update failure and the target-already-exists download failure. Lets clients render the dirty/conflicting directory without parsing it out of the human-readable message, which is unreliable for paths containing `:` (e.g. Windows drive letters).
6479ea7 to
34a856d
Compare
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
34a856d to
4d478a2
Compare
|
Does this integrate with #4550 at all? |
Eh, not really, no. But I agree with an implication that it should, let me do one more pass to see if I can align it with James' changes. |
4d478a2 to
6f59b11
Compare
…eat/dir_sequence_output # Conflicts: # src/griptape_nodes/exe_types/param_components/project_image_sequence_parameter.py
| for index in range(1, _MAX_VERSION_INDEX + 1): | ||
| probe_variables = {**entry_macro.variables, "_index": index, _ENTRY_VAR_NAME: 0} | ||
| resolve_result = GriptapeNodes.handle_request( | ||
| GetPathForMacroRequest(parsed_macro=entry_macro.parsed_macro, variables=probe_variables) | ||
| ) | ||
| if not isinstance(resolve_result, GetPathForMacroResultSuccess): | ||
| msg = f"Attempted to find available sequence version. Failed to resolve macro: {resolve_result.result_details}" | ||
| raise FileSequenceError(msg) |
| path = PurePosixPath(pattern) | ||
| fseq = _FSeq(path.name, pad_style=PAD_STYLE_HASH1) | ||
| width = fseq.zfill() | ||
| if width == 0: | ||
| return pattern | ||
| entry_part = f"{{entry:{width:02}}}" | ||
| new_name = fseq.basename() + entry_part + fseq.extension() | ||
| parent = str(path.parent) | ||
| return f"{parent}/{new_name}" if parent != "." else new_name |
scan() was deriving the fileseq pattern from the unresolved macro
template, leaving {_index:03} literally in the pattern and causing
every scan to return []. Now derives from the resolved absolute path.
build_versioned_sequence_destination and _create_with_versioning both
had a race between exists()-check and mkdir(); now mkdir(exist_ok=False)
is used to atomically claim the slot, with FileExistsError caught to
retry the next index.
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
hash_pattern_to_entry_macro was called before FilenameParts.from_filename,
causing stems like render_{entry:04} to appear literally in directory names.
Call from_filename on the original filename instead.
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…Directory and FileSequence components
Extract three shared helpers to eliminate copy-pasted boilerplate introduced alongside Directory and FileSequence: - `_resolve_macro_path` / `_aresolve_macro_path` (file.py): shared dispatch for GetPathForMacroRequest; callers supply an exc_factory to produce their domain-specific exception (FileLoadError or DirectoryError). Replaces the duplicated 10-line resolve blocks in `_resolve_file_path`, `_aresolve_file_path`, and `_resolve_dir_path`. - `ResolvedSituation` + `resolve_situation()` (project_file.py): encapsulates the GetSituationRequest lookup, policy mapping, and fallback logic that was copy-pasted across `from_situation()`, `_build_directory_destination_from_situation()`, and `_build_sequence_destination_from_situation()`. - `_attempt_map_to_project()` (project_file.py): wraps AttemptMapAbsolutePathToProjectRequest, used by both `_map_to_macro_file` and `_map_to_macro_directory`.
feat: add Directory and FileSequence output parameter types
Closes #4634
Context
Nodes that produce files currently express their output as a single
Filewith a resolved path. Two common output shapes weren't representable:frame_v001_0001.exr … frame_v001_0042.exr(ordialogue_v001_001.md … dialogue_v001_099.md) into a versioned directoryWhat changed
New types (
files/directory.py,files/file_sequence.py)Directory/DirectoryDestination— thin wrappers aroundMacroPath | str, parallel to the existingFile/FileDestination.DirectoryDestination.create()handlesCREATE_NEWversioning by incrementing_indexuntil a non-existent path is found, then mkdirs.FileSequence/FileSequenceDestination— represents a collection of numbered files identified by an entry-number pattern. Internally uses{entry:04}macro syntax; exposes industry-standard####notation (and all other fileseq token forms:%04d,@@@@,$F4) atproperty boundaries via the fileseq library.
FileSequenceDestination.entry(n)returns aFileDestinationfor writing a specific entry; the version index (_index) is resolved once at construction and held constant across all entries.FileSequence.scan()delegates toScanSequencesRequestrather than calling the internal scanner directly.New parameter components (
exe_types/param_components/)ProjectOutputParameter— new shared base class extracted fromProjectFileParameter, consolidating the common wiring: adding a file-path parameter, attaching an open-in-explorer button, and emitting the resolved path via node messages.ProjectFileParameternow extendsthis.
ProjectDirectoryParameter— mirrorsProjectFileParameter;build_directory()returns aDirectoryDestinationbacked by thesave_output_directorysituation.ProjectFileSequenceParameter—build_sequence()returns aFileSequenceDestinationbacked by thesave_file_sequence_entrysituation.Both new components support the cog-button pattern to auto-create and connect a settings node.
New situations (
default_project_template.py)save_output_directory:
{outputs}/{sub_dirs?:/}{node_name?:_}{dir_name}_v{_index:03}
save_file_sequence_entry:
{outputs}/{sub_dirs?:/}{node_name?:_}{file_name_base}_v{_index:03}/
{file_name_base}_v{index:03}{entry:04}.{file_extension}
Version number (
_v001,_v002, …) appears in both the directory name and sequence entry filenames. Padding defaults to 3 digits for versions, 4 for entries.Wire types:
"Directory"and"FileSequence"— new distinctoutput_typevalues so the UI can render them with appropriate icons/pickers.Important note
filesystem, to use new output types (any other libs use them?)