feat(cli): run agent evals (CUB-3667) - #11953
MikeNitsenko wants to merge 8 commits into
Conversation
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Automations to automatically generate PRs for you. |
|
Claude finished @MikeNitsenko's task in 2m 41s —— View job Review complete — 1 high, 2 medium, 3 low, posted inlineDetails
Findings
What holds up wellThe fail-closed shape of Review threadsThe listing returned 0 threads of mine — no prior threads, none resolved, none re-affirmed. All six findings posted fresh. Not verifiedDid not run |
076b401 to
96e41b7
Compare
|
Claude finished @MikeNitsenko's task in 4m 13s —— View job Review: 1 high, 2 medium, 3 low —
|
| # | Severity | Where | What |
|---|---|---|---|
| 1 | High | rust/cube-cli/src/commands/evals.rs:144 |
ensure_passed bails on pageInfo.hasNextPage, and the results GET never pages. Any suite bigger than the server's default page fails the gate while green, with no --first anywhere to widen it. |
| 2 | Medium | rust/cube-cli/src/commands/evals.rs:252-260 |
evals results silently prints a truncated page — no --first/--after, no hasNextPage signal. Every other list command in the CLI carries the pair. |
| 3 | Medium | rust/cube-cli/src/commands/evals.rs:33-35 |
--file borrows util::nonempty_path, whose refusal advises - for stdin. The path is server-side; - parses and ships verbatim as questionFile. |
| 4 | Low | docs-mintlify/reference/cli.mdx:348-357, :181 |
"evaluation" / EVALUATION_ID against docs-mintlify/CLAUDE.md's Evals terminology rule. The CLI's own help and error strings have it throughout too. |
| 5 | Low | rust/cube-cli/src/commands/evals.rs:104-113 |
Only completed/failed are terminal, with no note on whether that set is exhaustive. A cancelled run would burn the whole --timeout. deployments.rs:11-21 and dbt.rs:664 both document their sets. |
| 6 | Low | rust/cube-cli/src/commands/evals.rs:305-330 |
No test reaches the FAILED arm of ensure_passed, nor proves the status lowercasing at :104/:141. |
What's good
The fail-closed design is right and unusually well thought through: total == 0 is a failure rather than a vacuous pass, an unread page refuses to be graded rather than grading what it has, requires = "wait" keeps --timeout/--poll from looking honoured when they aren't, and --branch is registered in util.rs's refuse-an-empty-value list with a comment naming the exact CI case. Progress goes to stderr so --json stays one document. Reusing wait::poll gets transient-failure absorption and the deadline-bounded attempt for free. The module also stays refreshingly light on comments for this codebase — nothing to flag under the comment rules.
Not verified
Couldn't run cargo test / clippy — the sandbox declined the invocations, so the PR body's "102 passed" is unconfirmed from here. Findings 1 and 5 hinge on the enterprise API's behaviour (default results page size, and whether an eval run has terminal states beyond completed/failed), which isn't in this repo; both are framed so the answer decides the fix.
Review threads
The listing returned 0 threads of mine — no prior rounds on this PR. Nothing resolved, nothing re-affirmed, no duplicates to suppress.
96e41b7 to
551fa64
Compare
|
Claude finished @MikeNitsenko's task in 2m 42s —— View job 1 medium, 4 low — the fail-closed grading logic is sound; the medium is that Full review
Findings
What holds up well
Review threadsThe thread listing for this PR returned 0 threads of mine (empty, single page), so nothing to resolve and no finding suppressed as a duplicate. The five review comments visible in the PR description context are MikeNitsenko's own, which I do not touch. Not verified
|
|
Claude finished @MikeNitsenko's task in 3m 1s —— View job Reviewed — no blockers. 0 high, 1 medium, 2 low; 3 inline comments posted. Review details
Findings
What looks right
Notes
|
|
Claude finished @MikeNitsenko's task in 4m 37s —— View job Reviewed — no blockers. 3 findings posted inline (1 medium docs, 2 low). Review detail
Findings
What holds up wellThe CI gate is genuinely fail-closed: Moving
Notes
Review threadsThe listing returned 0 of my own threads (paged to the end; no unresolved threads of any author on this PR). 0 resolved, 0 re-affirmed. All three findings above are new. |
|
Claude finished @MikeNitsenko's task in 2m 43s —— View job 1 medium finding — zero-length results-fetch budget when a run finishes at the Review details
Finding
Checked and not flagged
Review threadsThe listing returned 0 threads of mine on this PR — no prior threads, so none resolved and none re-affirmed. |
|
Claude finished @MikeNitsenko's task in 2m 47s —— View job No blocking findings — 0 high, 0 medium, 1 low (non-blocking note)Review details
Review threadsThe listing returned 0 unresolved threads (0 of mine); 0 resolved, 0 re-affirmed. Nothing to dedupe. What I verifiedFail-closed gate — Envelope consistency — Timeout budget — Terminal safety — every server-supplied cell now goes through Stream discipline — the started-run notice ( Arg validation — Docs — Low (non-blocking)
Not run
|
|
Claude finished @MikeNitsenko's task in 3m 51s —— View job Review complete — 0 blocking, 3 low (all docs/help text); Rust logic looks soundFull review
Inline findings
What I checked and found clean
Not verified
Review threadsListing returned 0 threads (one page, |
|
Claude finished @MikeNitsenko's task in 4m 6s —— View job Review complete — no blocking findings (0 high, 0 medium, 1 low observation, not posted inline)Full review
Review threadsThe listing returned 0 of my own threads ( What I checked and found cleanFail-closed gating — Timeout arithmetic — Retry semantics — Terminal safety — Pagination hint — Docs — the One low observation (not worth an inline comment)
Not verifiedI could not run the Rust test suite — |
Summary
cube evals run,status, andresults--waitbehavior that fails closed unless every graded question passesDepends on the public API from cubedevinc/cubejs-enterprise#15265.
Verification
cargo +1.96.0 fmt --all --checkcargo +1.96.0 clippy --all-targets -- -D warningscargo +1.96.0 test --locked(103 passed)