Skip to content

refactor(editor): extract export status model#566

Merged
meiiie merged 2 commits into
mainfrom
refactor/editor-export-status-model
May 22, 2026
Merged

refactor(editor): extract export status model#566
meiiie merged 2 commits into
mainfrom
refactor/editor-export-status-model

Conversation

@meiiie
Copy link
Copy Markdown
Collaborator

@meiiie meiiie commented May 22, 2026

Description

Extracts the export progress/status display model from VideoEditor.tsx into a small pure module, exportStatusModel.ts.

Motivation

VideoEditor.tsx still owns several unrelated export responsibilities. This slice keeps reducing that surface area by moving derived export UI state into a testable boundary without touching export execution, native routing, save behavior, or renderer lifecycle.

Type of Change

  • Refactor
  • Tests

Related Issue(s)

None.

Changes Made

  • Added resolveExportStatusModel for export progress/status derived state.
  • Moved runtime path labels, native skip labels, finalization flags, progress mode flags, and render-speed visibility into the new pure model.
  • Added focused unit tests covering preparing, legacy/modern MP4 progress, finalization, audio finalization, render speed visibility, runtime labels, and native skip labels.
  • Updated VideoEditor.tsx to consume the model while keeping i18n text composition in the component.

Scope Note

This PR does not change export behavior, UX copy, native helper routing, NVIDIA detection, save/finalize behavior, renderer selection, project persistence, or packaged build settings. It is a pure extraction of existing derived UI state.

Testing Guide

  1. Open the editor.
  2. Start an MP4 export and confirm progress/status UI still behaves as before.
  3. Optional: test both Lightning and legacy MP4 export modes to confirm the secondary hints still appear correctly.

Checklist

  • Focused branch from latest main
  • Small architecture slice only
  • Added/updated tests
  • Typecheck passes
  • Scoped Biome passes
  • No runtime/export behavior intentionally changed

Local Checks

  • npm test -- src/components/video-editor/exportStatusModel.test.ts src/components/video-editor/mp4ExportSettings.test.ts src/components/video-editor/mp4ExportRouting.test.ts src/components/video-editor/useNvidiaCudaExportOptIn.test.ts
  • npx tsc --noEmit --pretty false
  • npx biome check --formatter-enabled=false src/components/video-editor/VideoEditor.tsx src/components/video-editor/exportStatusModel.ts src/components/video-editor/exportStatusModel.test.ts
  • git diff --check

Runtime/Repro Evidence

Not run. This is a pure derived-state extraction with no exporter/native/runtime path changes.

Summary by CodeRabbit

  • Refactor

    • Improved export progress and status reporting for more accurate, user-facing export phase indicators (saving/preparing/finalizing)
    • Better detection and display of audio rendering state, render speed label, runtime/encoder labeling, and native-skip summaries
    • Adjusted progress bar calculations to handle audio progress safely
  • Tests

    • Added comprehensive tests covering multiple export scenarios and status label/flag behaviors

Review Change Stack

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 22, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 830cc591-30a2-43e2-a995-029b580f0107

📥 Commits

Reviewing files that changed from the base of the PR and between ac24952 and 3ad40f7.

📒 Files selected for processing (2)
  • src/components/video-editor/exportStatusModel.test.ts
  • src/components/video-editor/exportStatusModel.ts

📝 Walkthrough

Walkthrough

A new exportStatusModel module centralizes derivation of UI-ready export status fields from export state, format, pipeline model, and progress values. VideoEditor.tsx is refactored to use the model instead of computing these fields inline. A Vitest suite validates the model across multiple export scenarios.

Changes

Export Status Model Extraction

Layer / File(s) Summary
Export-status model and runtime labeling
src/components/video-editor/exportStatusModel.ts
ExportStatusModel type and resolveExportStatusModel compute phase flags (saving/preparing/finalizing), audio-rendering detection, clamped/rounded finalizing progress/percent, export mode flags (muxing/saving, lightning vs legacy), shouldSuspendPreviewRendering, conditional renderSpeedFps, native skip-reason fields/label, and runtimeLabel via resolveRuntimeLabel.
Export-status model validation
src/components/video-editor/exportStatusModel.test.ts
Vitest suite with a progress() helper validating model outputs across modern/legacy MP4, initial preparing state, finalizing progress derivation and clamping, separation of audio finalization from muxing/saving, render-FPS visibility by phase, runtime label composition/fallback to encoderName, and native skip-reason aggregation/label formatting.
VideoEditor refactoring and safety fix
src/components/video-editor/VideoEditor.tsx
VideoEditor imports resolveExportStatusModel and replaces inline export-status derivations with the model output; exportRenderSpeedLabel now uses model renderSpeedFps. Progress-bar width now reads exportProgress?.audioProgress with optional chaining for safety.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • webadderallorg/Recordly#262: Touches export UI finalizing percent and audio-export rendering state in VideoEditor.tsx, overlapping progress/percent logic moved into the model.
  • webadderallorg/Recordly#452: Also modifies VideoEditor.tsx export-phase/progress UI logic and overlaps the same export-progress labeling code paths.

Suggested labels

Checked

Poem

🐰 I hopped through lines and joined the threads,
Gathered export flags and progress heads,
A model now holds the scattered clues,
Tests confirm the numbers and the views,
Hooray — the editor’s tidy, too!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'refactor(editor): extract export status model' is specific and directly describes the main change in the PR—extracting export status model logic into a separate module.
Description check ✅ Passed The PR description is comprehensive and addresses most template sections with clear explanations, motivation, type of change, related changes, and testing guidance.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch refactor/editor-export-status-model

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@meiiie meiiie marked this pull request as ready for review May 22, 2026 15:11
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/components/video-editor/exportStatusModel.ts`:
- Around line 38-48: exportFinalizingProgress currently only upper-clamps and
doesn't guard against NaN/negative/infinite values; update the computation in
the exportFinalizingProgress/exportFinalizingPercent logic (referencing
exportFinalizingProgress, exportFinalizingPercent,
exportProgress.renderProgress, exportProgress.percentage, isExportFinalizing) to
first derive a numeric rawProgress (use renderProgress if typeof number else
exportProgress?.percentage ?? 100), then sanitize by treating non-finite values
as 0 and clamping with Math.max(0, Math.min(100, rawProgress)), and finally
compute exportFinalizingPercent by rounding that sanitized/clamped value; ensure
rounding happens after sanitization.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 36113b2d-c354-4b55-ac6e-802a7955ad43

📥 Commits

Reviewing files that changed from the base of the PR and between 6cc0712 and ac24952.

📒 Files selected for processing (3)
  • src/components/video-editor/VideoEditor.tsx
  • src/components/video-editor/exportStatusModel.test.ts
  • src/components/video-editor/exportStatusModel.ts

Comment thread src/components/video-editor/exportStatusModel.ts
@meiiie meiiie merged commit 9c29d4c into main May 22, 2026
3 checks passed
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.

1 participant