[misc] Codebase structure audit: ranked P1/P2 changes

Open
  • Is something wrong?
#2,074 1 comment 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 2 days

Nobody has claimed this yet.

Assessment

Difficulty
5/5
Estimated time
Over a week
Newbie friendliness
25/100
Issue type
Refactor
Clarity
Mostly clear
Activity status
Active
Tech stack
python
Domain
tooling

Research direction

Start with the ranked P1/P2 sections and inspect the named files, especially the contract test pattern in fastvideo/tests/contract/test_env_policy.py and the related work in #1254, #1913, and #1698. The audit lists multiple independent changes, including behavior-sensitive model loading work and a boundary change that needs a maintainer decision; choose one scoped item before starting. Done means the selected change meets its stated checks, such as a shrink-only allowlist, loader tests and SSIM coverage, or a parity test.

Written by the indexing model from the issue text.

Description

installation platform scope: attention scope: data scope: distributed scope: inference scope: model scope: training type: refactor

Audit of FastVideo against the untangle-codebase lenses (hidden links, needless links, surplus, special cases). Static analysis only; no code changed. Ranked by payoff. P1 items are behavior-neutral under the repo's small-diff and SSIM/performance rules.

P1

1. Enforce the two-stack training boundary

fastvideo/AGENTS.md says imports between fastvideo/train/ and fastvideo/training/ are forbidden. 13 files in fastvideo/train/ import fastvideo/training/ today: callbacks/ema.py, callbacks/posthoc_ema.py, callbacks/validation.py, methods/base.py, methods/rl/diffusion_nft.py, models/cosmos/cosmos.py, models/hunyuan/hunyuan.py, models/hunyuan15/hunyuan15.py, models/kandinsky5/kandinsky5.py, models/matrixgame2/matrixgame2.py, models/wan/wan.py, utils/optimizer.py, utils/tracking.py. The modular trainer's EMA, checkpointing, optimizer/scheduler and tracking paths depend on the frozen legacy stack, so a legacy change breaks them silently. Add fastvideo/tests/contract/test_training_stack_boundary.py using the shrink-only allowlist pattern already in fastvideo/tests/contract/test_env_policy.py: new coupling fails, and fixing an allowlisted site requires deleting its entry. Test-only, no runtime behavior.

2. Stop copying the denoising verify_input check sets

Three distinct check sets are each duplicated, 7 methods total across 6 files: pipelines/basic/wan/stages/causal_denoising.py:117, pipelines/stages/matrixgame2_denoising.py:730, pipelines/stages/matrixgame3_denoising.py:36 (one set); pipelines/stages/latent_preparation.py:150 and pipelines/basic/ltx2/stages/ltx2_latent_preparation.py:269; pipelines/stages/denoising.py:670 and pipelines/stages/sr_denoising.py:322. The bodies are AST-identical. Move each set into one module-level helper in fastvideo/pipelines/stages/ and call it from each method. The checks are unchanged, so verification output is identical. Adding or changing an input check then happens once instead of up to three times.

P2

3. Replace model-name branches in shared code with declared capabilities

models/loader/component_loader.py branches on Cosmos25 and Gen3C (:881-886), MMAudioVAE and Kandinsky6AudioVAE (:1108-1119), Cosmos25 again (:1302-1303), MiniMaxH3 (:1329) and LTX2 (:1534); models/loader/fsdp_load.py:533 branches on MiniMaxH3; pipelines/stages/decoding.py:87,176 branches on LingBotVideo; component_loader.py:1311 lazy-imports a specific pipeline. Each is a requirement the shared path never modeled, the same class as #1254. Declaring these as per-component capabilities removes the branches and the models -> pipelines import. Stage one capability at a time with the loader tests and an SSIM run; it touches loading, so it is not P1.

4. Move the shared primitives out of the legacy stack

fastvideo/train/ pulls about 19 symbols from fastvideo/training/: EMA_FSDP, normalize_dit_input, compute_density_for_timestep_sampling, get_sigmas, shift_timestep, ModelWrapper/OptimizerWrapper/SchedulerWrapper/get_scheduler, Trackers/initialize_trackers, and checkpoint helpers. Relocating them into the new stack, or a neutral module, lets the legacy stack import from the new one, and then the allowlist from item 1 burns down. Needs a maintainer call because it crosses the two-stack boundary.

5. Keep the MLX and CUDA H3 reference paths in sync

fastvideo/mlx_runtime/minimax_h3_reference.py and fastvideo/pipelines/basic/minimax_h3/reference.py share sample_reference_video_frames and build_ref2va_presentation. Share one source or add a parity test; today the two can drift.

6. Decide per family on the forked model ports

Large identical bodies exist between sibling ports: cosmos2_5/gen3c, hunyuanvideo/hunyuanvideo15, kandinsky5/kandinsky6, lingbotworld/lingbotworld2, matrixgame2/matrixgame3, clip/siglip. For each family, accept the fork as intentional or share a base. Do not refactor model internals without SSIM coverage for that model.

7. Small convention cleanups

Replace the remaining direct logging.getLogger calls with init_logger(__name__) in non-excluded files (eval/worker.py, train/methods/knowledge_distillation/kd.py, worker/multiproc_executor.py). The three overfit preprocess scripts (pipelines/preprocess/preprocess_cosmos25_overfit.py, preprocess_cosmos_overfit.py, preprocess_hunyuan_overfit.py) share an identical load_video.

Not doing

  • The skill's "fix everything until nothing left" loop. It conflicts with the SSIM/performance gates, the small-diff review rule, and the freeze on legacy pipelines.
  • The skill's Tests option. Deleting tests that "can't fail" conflicts with the repo rule against weakening tests.
  • A wholesale chase of import cycles. There are 27 module-level 2-cycles and one 202-module SCC (configs 103, pipelines 37, models 21, rest smaller); most are registry/registration patterns. Document, do not rewrite.
  • A naive dead-code sweep. Eval metrics and model classes resolve by name, so a text scan reports 157 modules as unreferenced; that number is not a work list.
  • Reformatting pre-commit-excluded dirs (third_party/, dataset/, models/).

Method: AST import graph over 769 non-test modules, AST-body hashing for function-level duplicates (34 groups at 8 statements or more), and targeted greps. Related work: #1254, #1913, merged #1698.

— fv-superagent · run by @aryan5v

Dominant language
Python
Stars
4.6k
Forks
486
Avg merge
3d 15h
Merged PRs (30d)
74

Getting set up

  • No Dockerfile or Docker Compose file
  • Has a pull request template
  • No contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from hao-ai-lab/FastVideo

All issues in hao-ai-lab/FastVideo

Similar issues

More Python issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.