Superseded by #9 — workload contract - #7
Conversation
Introduce the shared generation workload schema that MotionKernel and FastVideo use for baseline measurement, profiling, and end-to-end validation. Ship Wan and LTX manifests, result classification, a resume-friendly launcher bridge, and CPU tests. GPU execution depends on the paired FastVideo generation launcher.
|
Warning Review limit reached
Next review available in: 53 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughAdded validated workload and generation-result schemas, FastVideo native/optimized launcher orchestration, resumable state handling, comparison classification, CLI commands, canonical LTX and Wan manifests, tests, and optimization workflow documentation. ChangesWorkload execution foundation
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Operator
participant workload.py
participant run_ab
participant FastVideo
participant ResultStore
Operator->>workload.py: run-ab workload
workload.py->>run_ab: invoke native and optimized modes
run_ab->>FastVideo: execute launcher command
FastVideo-->>run_ab: write generation result
run_ab->>ResultStore: load and classify results
ResultStore-->>workload.py: comparison JSON
workload.py-->>Operator: print structured run result
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (2)
tests/test_workload.py (1)
76-80: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSuppress the S105 false positive.
Ruff reports
S105for the literal on line 78. The literal is a rejection fixture, not a credential. Add a suppression so lint stays clean.🧹 Proposed suppression
- payload["runtime"]["password"] = "nope" + payload["runtime"]["password"] = "nope" # noqa: S105 - rejection fixture🤖 Prompt for 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. In `@tests/test_workload.py` around lines 76 - 80, Add a narrowly scoped Ruff S105 suppression to the `"nope"` rejection-fixture assignment in `test_rejects_secret_fields`, preserving the test’s behavior while preventing this intentional non-credential literal from triggering lint.Source: Linters/SAST tools
autokernel/workload/launcher.py (1)
261-263: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winWrite comparison.json atomically.
The state file and the result files use a
.tmpfile plusreplace. This write does not. An interrupted write leaves a partialcomparison.json, and the resume branch on line 268 then fails to parse it. Reuse the same atomic pattern.🤖 Prompt for 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. In `@autokernel/workload/launcher.py` around lines 261 - 263, Update the comparison.json write in the launcher flow to use the same temporary-file-and-replace pattern as the state and result file writes. Write the serialized comparison data to a .tmp file with the existing encoding, then atomically replace comparison_path so interrupted writes cannot leave a partial file for the resume branch.
🤖 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 `@autokernel/workload/launcher.py`:
- Around line 52-59: Update _read_state to catch JSON parsing failures and
validate that the decoded value is an object with the expected schema_version
and required state fields before returning it. For missing, malformed,
foreign-schema, or incorrectly shaped state files, raise WorkloadError so
run_ab’s existing error handling reports the failure without a traceback.
- Around line 203-211: Validate every entry in modes against the accepted STAGES
set before entering the launcher loop, raising WorkloadError for any unknown
mode. Update the mode-dependent result-path and comparison logic around the loop
and comparison block so only validated stage names are processed and invalid
values cannot produce an unexplained null comparison.
- Line 194: Update run_ab so each run_mode invocation passes the selected mode’s
environment via manifest.mode_env.for_mode(mode), using the mode being launched
for both comparison paths. Ensure the env argument reaches each launcher
subprocess so Manifest.mode_env settings differentiate native and optimized
runs.
In `@autokernel/workload/result.py`:
- Line 15: Move the shared validators, including _finite_number, _mapping,
_text, _optional_text, _non_negative_int, and _positive_int, into a common
internal validation module and import them from both types and result. Update
the validator API to accept the caller-specific message prefix, then pass the
appropriate prefix from result so generation-result errors identify the correct
source instead of using workload-specific text. Remove the duplicated validator
implementations from result while preserving existing validation behavior.
- Around line 302-312: Update the validation guard before the speedup
calculation in the result-classification flow to reject
optimized.median_wall_seconds values less than or equal to zero, alongside the
existing native check. Preserve the "failed" classification and "missing median
wall times" reason, and ensure speedup is only computed with a positive
optimized divisor.
- Around line 69-88: Update _number_list so None entries are not removed from
wall_seconds: either reject them through validation or preserve their positions
with the appropriate nullable result type. Ensure wall_seconds remains
index-aligned with generation_seconds and peak_memory_mb, while retaining
existing finite-number validation for non-None samples.
In `@docs/FASTVIDEO_UNIVERSAL_OPTIMIZATION_AGENT_PLAN.md`:
- Around line 17-24: Replace the developer-specific absolute paths in the
referenced documentation with repository-relative paths where applicable, or
placeholders such as <motionkernel-checkout> and <fastvideo-checkout> for
external checkout locations. Preserve the existing references to the
optimization guide, Wan measurement script, and results document without
exposing local usernames.
---
Nitpick comments:
In `@autokernel/workload/launcher.py`:
- Around line 261-263: Update the comparison.json write in the launcher flow to
use the same temporary-file-and-replace pattern as the state and result file
writes. Write the serialized comparison data to a .tmp file with the existing
encoding, then atomically replace comparison_path so interrupted writes cannot
leave a partial file for the resume branch.
In `@tests/test_workload.py`:
- Around line 76-80: Add a narrowly scoped Ruff S105 suppression to the `"nope"`
rejection-fixture assignment in `test_rejects_secret_fields`, preserving the
test’s behavior while preventing this intentional non-credential literal from
triggering lint.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 281540fe-893d-4f9e-9ad5-1d41597897f5
📒 Files selected for processing (10)
autokernel/workload/__init__.pyautokernel/workload/launcher.pyautokernel/workload/result.pyautokernel/workload/types.pydocs/FASTVIDEO_UNIVERSAL_OPTIMIZATION_AGENT_PLAN.mdpyproject.tomltests/test_workload.pyworkload.pyworkloads/ltx_480p.yamlworkloads/wan_t2v_1.3b_480p.yaml
Validate launcher resume state JSON, pass mode_env into child processes, reject unknown A/B modes, guard zero optimized medians, reject null wall_seconds, share validators via _validate, and drop machine-local paths from the plan doc.
|
Addressed CodeRabbit findings in 634d38c:
Left unmerged until review is fully clean. |
aryan5v
changed the title
1 participant
Superseded by #9, which contains this workstream plus the reviewed fixes, profiler ingestion, parity enforcement, FX capture, and impact ranking.