feat(bench): support multiple transforms and --all flag#90
feat(bench): support multiple transforms and --all flag#90
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
There was a problem hiding this comment.
1 issue found across 2 files
Confidence score: 4/5
- This PR is likely safe to merge, with only a small CLI behavior gap rather than a crash or data-loss risk (severity 4/10).
- In
src/main.rs, allowing--alland--transformtogether currently causes--allto silently win, which can confuse users and produce unintended command behavior. - Pay close attention to
src/main.rs- enforce mutual exclusivity for--alland--transformso invalid combinations fail fast.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/main.rs">
<violation number="1" location="src/main.rs:251">
P2: `--all` and `--transform` can be passed together, and `--all` silently wins. Make them mutually exclusive so invalid combinations fail fast.</violation>
</file>
Architecture diagram
sequenceDiagram
participant U as User (CLI)
participant M as Main (main.rs)
participant B as Benchmark Service (benchmark.rs)
participant T as Transform Engine
U->>M: vuke bench [--all] [--transform X] [--json]
Note over M: NEW: Resolve Transform List
alt --all flag provided
M->>B: NEW: default_transforms()
B-->>M: Return List (Sha256, Md5, etc.)
else Specific transforms provided
M->>M: Use provided Vec<TransformType>
else No flags
M->>M: Default to [Sha256]
end
M->>B: CHANGED: run_benchmark(transforms, json)
loop For each Transform in list
B->>B: NEW: bench_single(transform)
B->>T: create()
T-->>B: transform instance
Note over B: Run 2s warmup + 5s measurement
B->>B: internal execution & counter update
end
alt json == true
Note over B: NEW: Return JSON array if multiple
B-->>U: Output JSON string/array
else transforms.len() == 1
Note over B: Behavior unchanged for single
B-->>U: Output single result stats
else transforms.len() > 1
Note over B: NEW: Comparison Table
B-->>U: Output tabular throughput comparison
end
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| transform: Vec<TransformType>, | ||
|
|
||
| /// Benchmark all transforms | ||
| #[arg(long)] |
There was a problem hiding this comment.
P2: --all and --transform can be passed together, and --all silently wins. Make them mutually exclusive so invalid combinations fail fast.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/main.rs, line 251:
<comment>`--all` and `--transform` can be passed together, and `--all` silently wins. Make them mutually exclusive so invalid combinations fail fast.</comment>
<file context>
@@ -243,9 +243,13 @@ enum Command {
+ transform: Vec<TransformType>,
+
+ /// Benchmark all transforms
+ #[arg(long)]
+ all: bool,
</file context>
| #[arg(long)] | |
| #[arg(long, conflicts_with = "transform")] |
Closes #83
vuke benchused to take a single--transform. Comparing throughput across transforms meant running the command a dozen times. Now it accepts multiple values and an--allflag:Multiple transforms print a comparison table.
--allincludes all benchmarkable transforms (everything except bitimage which needs external files). JSON output returns an array when benchmarking multiple transforms.Single transform behavior is unchanged.