[codex] support multi-folder imports and series numbering - #51
Sudo-Rahman wants to merge 4 commits into
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e37571326c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const ruleTypes: RuleType[] = [ | ||
| 'prefix', 'suffix', 'replace', 'regex', | ||
| 'remove', 'case', 'number', 'move', 'timestamp', | ||
| 'remove', 'case', 'number', 'series-number', 'move', 'timestamp', |
There was a problem hiding this comment.
When this rule is added from the shared RenameWorkspace used by Merge or Transcode output naming, a group with no detected season—or conflicting season evidence—is marked by the store as hasBlockingIssues, but MergeView.handleMerge and TranscodeView.handleStartTranscode only check filename conflicts. Those operations therefore remain executable and applySeriesNumber silently falls back to season 1, producing incorrectly named merged/transcoded files. Propagate hasBlockingIssues into both embedded execution guards, or restrict the rule to workspaces that honor it.
Useful? React with 👍 / 👎.
| function normalizeSeason(value: number | undefined): number | undefined { | ||
| return value !== undefined && Number.isInteger(value) && value > 0 ? value : undefined; |
There was a problem hiding this comment.
For files using the standard specials notation S00E01 or a Season 0 folder, the parser produces season 0, but this predicate discards it; setSeasonAssignment and the editor's min="1" also prevent correcting it manually. The group consequently remains unresolved, blocking the main Rename workflow and triggering the season-1 fallback in embedded workflows, so specials cannot be assigned the correct S00 prefix.
Useful? React with 👍 / 👎.
| function handlePaddingChange(event: Event): void { | ||
| const target = event.target as HTMLInputElement; | ||
| onUpdate({ ...config, padding: parseInt(target.value, 10) || 1 }); |
There was a problem hiding this comment.
When a user types or pastes a value outside the declared 1–10 range, the number input's min/max attributes do not clamp it, and this handler stores the unchecked integer. The reactive preview immediately passes that value to padStart; a sufficiently large value throws RangeError: Invalid string length and can break the rule dialog, while smaller out-of-range values generate unusable filenames. Clamp the parsed value to the same bounds advertised by the input before calling onUpdate.
Useful? React with 👍 / 👎.
…ded flows - Block unresolved series rules in embedded Merge and Transcode output flows - Allow season zero for specials in parser, store, and UI inputs - Clamp episode padding to 1-10 across rule editors and rename preview/apply logic
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0c3c294023
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| {selectedTracksCount} | ||
| </Badge> | ||
| </Item.Root> | ||
| {#if workspace.hasSeriesNumberingIssues} |
There was a problem hiding this comment.
For ordinary movie files with no detectable season, hasSeriesNumberingIssues is true even when the workspace has no enabled Series Numbering rule, so this renders a destructive “Needs selection” warning for an irrelevant feature while merging remains allowed. The same unconditional check appears in TranscodeOutputNamingView.svelte and TranscodeOutputTab.svelte; gate these warnings on an enabled series rule, consistent with hasBlockingIssues.
Useful? React with 👍 / 👎.
|
|
||
| function handleStepChange(event: Event): void { | ||
| const target = event.target as HTMLInputElement; | ||
| onUpdate({ ...config, step: parseInt(target.value, 10) || 1 }); |
There was a problem hiding this comment.
When a user pastes a negative value such as -1, the input's min="1" does not sanitize it and this handler stores it because negative integers are truthy. Series numbering then decreases from the configured start and eventually generates tokens such as S01E0-1; clamp the parsed step to at least 1 before updating the rule.
Useful? React with 👍 / 👎.
| const activeGroupKeys = new Set( | ||
| planSeriesNumbering(sourceFiles, sortConfig, seasonAssignments).resolutions.map((resolution) => resolution.groupKey), | ||
| ); |
There was a problem hiding this comment.
When the last file in a manually assigned group is temporarily deselected, planSeriesNumbering returns only selected groups, so this pruning pass deletes the assignment even though the group's files remain in the workspace. Reselecting the file then silently restores the unresolved blocker and forces the user to enter the season again; prune against groups represented by all remaining files rather than the current selection.
Useful? React with 👍 / 👎.
| async function handleAddTrackFolders(): Promise<void> { | ||
| await addFiles(await pickAndExpandToolImport(trackImportPolicy, 'merge', 'folders')); |
There was a problem hiding this comment.
When a selected track folder or mixed folder drop contains a matching .idx/.sub pair, recursive expansion returns both paths and addFiles creates a separate imported track for each. These files are the two parts of one VobSub subtitle, but downstream merge construction passes every attached track as its own FFmpeg input, so auto-matching or attaching all imported tracks can duplicate the subtitle or make the merge fail; collapse each matching pair into one logical import before calling addFiles.
Useful? React with 👍 / 👎.
| return; | ||
| } | ||
|
|
||
| if (outputNamingWorkspace.hasBlockingIssues) { |
There was a problem hiding this comment.
The new guard evaluates hasBlockingIssues across every selected output-workspace file, while this function processes only selectedVideosToMerge; therefore an unresolved series group belonging to a scanning or failed video can block otherwise ready videos from merging. The Transcode guard has the same mismatch with readyQueueFiles; compute the series blocker from the actual execution subset, as the preceding scoped conflict calculation already does.
Useful? React with 👍 / 👎.
… and scoped blockers
1 participant
Summary
MediaFlow previously accepted files only, which forced users importing multi-season media to reopen the picker for every season. The rename workspace also numbered every imported file as one global sequence, so season numbers had to be maintained manually by repeatedly selecting and editing subsets.
This pull request adds multi-folder import across the supported tools while preserving each tool's file-format policy. Imported files retain source-group provenance, allowing the new Series Numbering rule to assign seasons per selected folder and restart episode numbering for each group.
Root cause and user impact
Import flows were implemented separately in each tool and passed only flat file paths. There was no shared representation of selected roots or source groups, so downstream rename logic could not distinguish files originating from different season folders. Standard numbering therefore treated the entire batch as one sequence.
The initial UI integration also exposed a few usability regressions: drag-and-drop zones stopped opening the picker on click, long source-folder names could force horizontal scrolling in the rule dialog, secondary Merge track imports inherited the wrong label, and saving a loaded rename preset always created a new preset.
Changes
Importfor primary imports while preserving contextual secondary actions such asAdd tracks.SaveandSave As: Save updates the active user preset, Save As creates and activates a new preset, and built-in presets remain immutable.Validation
pnpm check— 0 errors (one existing missing Node type-definition warning)pnpm test— 87 files and 712 tests passedpnpm build— passedcargo test --manifest-path src-tauri/Cargo.toml— 439 passed, 3 ignored; build-support tests 8 passedcargo clippy --all-targets --no-default-features— passed with existing warnings outside this change