You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
If anything inside fails, the bundle is still written, run.log never records that --stats was requested, and the process exits 0. The user asked for statistics, gets a bundle without them, and nothing in the output says so.
Both outcomes are also near-invisible at default verbosity: the success line is logger.info, hidden without -v. So "worked" and "silently didn't" look similar unless you go looking in the bundle itself.
Note too that return (table if table.num_rows else None) means a statistics table computed with zero rows also collapses to None on the success path — indistinguishable downstream from the failure path.
Why it matters
--stats is not incidental. Projection statistics are a headline capability, and for anyone quoting a separability or cluster-agreement number the statistics part is the entire result. A silent no-op here means:
for an interactive user: a bundle that looks fine and simply lacks the feature they asked for;
for a scripted/CI run: no signal at all, because the exit code is 0;
for anyone publishing: the failure mode is discovering afterwards that the numbers were never computed.
This is especially awkward at scale, which is exactly where --stats is most likely to fail (it is documented as "can be slow on large runs") and least likely to be spot-checked.
Suggested fix
Options, in increasing strictness — any of them would be an improvement:
At minimum: raise the log level to logger.error, and record in run.log that statistics were requested and whether they were produced. A reproducibility log that omits a requested-but-failed stage is asserting something false, same shape as run.log prints applied and unapplied projection parameters side by side #432.
Better: narrow the except to the failure classes actually anticipated, so an unexpected error surfaces instead of being folded into "statistics are secondary".
Best, and consistent with the CLI's existing convention: fail the run when --stats was explicitly requested and produced nothing. cli/prepare.py already translates ValueError into a clean ERROR: <msg> + typer.Exit(1). An explicitly requested stage that silently produces nothing is a failure; an implicit one arguably isn't.
Distinguishing "requested and failed" from "requested and legitimately empty" would also help — right now both return (None, {}).
Context
Found while auditing the statistics path before relying on it for published numbers. It is the same shape as PRs #430 and #431 and issue #432: an operation that did not happen reporting identically to one that did. In our case the guard was external — we now verify the statistics part exists in the bundle rather than trusting the exit code.
Summary
pipeline.py(~905-907) wraps the whole statistics computation in a bareexcept Exception:If anything inside fails, the bundle is still written,
run.lognever records that--statswas requested, and the process exits 0. The user asked for statistics, gets a bundle without them, and nothing in the output says so.Both outcomes are also near-invisible at default verbosity: the success line is
logger.info, hidden without-v. So "worked" and "silently didn't" look similar unless you go looking in the bundle itself.Note too that
return (table if table.num_rows else None)means a statistics table computed with zero rows also collapses toNoneon the success path — indistinguishable downstream from the failure path.Why it matters
--statsis not incidental. Projection statistics are a headline capability, and for anyone quoting a separability or cluster-agreement number the statistics part is the entire result. A silent no-op here means:This is especially awkward at scale, which is exactly where
--statsis most likely to fail (it is documented as "can be slow on large runs") and least likely to be spot-checked.Suggested fix
Options, in increasing strictness — any of them would be an improvement:
logger.error, and record inrun.logthat statistics were requested and whether they were produced. A reproducibility log that omits a requested-but-failed stage is asserting something false, same shape as run.log prints applied and unapplied projection parameters side by side #432.exceptto the failure classes actually anticipated, so an unexpected error surfaces instead of being folded into "statistics are secondary".--statswas explicitly requested and produced nothing.cli/prepare.pyalready translatesValueErrorinto a cleanERROR: <msg>+typer.Exit(1). An explicitly requested stage that silently produces nothing is a failure; an implicit one arguably isn't.Distinguishing "requested and failed" from "requested and legitimately empty" would also help — right now both return
(None, {}).Context
Found while auditing the statistics path before relying on it for published numbers. It is the same shape as PRs #430 and #431 and issue #432: an operation that did not happen reporting identically to one that did. In our case the guard was external — we now verify the statistics part exists in the bundle rather than trusting the exit code.