From aa9e5ad1d55cf6b82127f080be0f30a0c2c9f7e7 Mon Sep 17 00:00:00 2001
From: Karl Ding <karlding@users.noreply.github.com>
Date: Mon, 20 Oct 2025 13:07:41 -0700
Subject: fix: stats ngram window size cli parsing (#2946)

Fix panic when running "atuin stats" with an empty window size:

    $ RUST_BACKTRACE=1 atuin stats year -n0

thread 'main' panicked at crates/atuin-history/src/stats.rs:278:14:
      window size must be non-zero
      stack backtrace:
         0: __rustc::rust_begin_unwind
         1: core::panicking::panic_fmt
         2: core::option::expect_failed
         3: atuin_history::stats::compute
         4: atuin::command::client::Cmd::run_inner::{{closure}}
         5: tokio::runtime::context::scoped::Scoped<T>::set
6: tokio::runtime::scheduler::current_thread::CoreGuard::block_on
         7: tokio::runtime::context::runtime::enter_runtime
         8: atuin::command::AtuinCmd::run
         9: atuin::main
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a
      verbose backtrace.

The ngram window size had a default value of 1 (indicating that perhaps
this case was considered), but did not validate the parsed value when a
non-default value was given.

Such validation can either occur:

1. during clap argument parsing
2. after clap parses arguments, but before computing statistics

If we were to do this after clap finishes parsing arguments, this would
be a runtime check in run(), but it seems better to return an error as
soon as possible.

It's possible to do this without a custom parsing function via
clap::value_parser! and specifying a range:

    clap::value_parser!(u64).range(1..)

However, this does not appear to support custom error messages so we
would be left with the default message specifying an invalid range.

Therefore, add a custom parser function and return an error when the
value cannot be parsed as a valid ngram value.

<!-- Thank you for making a PR! Bug fixes are always welcome, but if
you're adding a new feature or changing an existing one, we'd really
appreciate if you open an issue, post on the forum, or drop in on
Discord -->

## Checks
- [x] I am happy for maintainers to push small adjustments to this PR,
to speed up the review cycle
- [x] I have checked that there are no existing pull requests for the
same thing
---
 crates/atuin/src/command/client/stats.rs | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

(limited to 'crates')

diff --git a/crates/atuin/src/command/client/stats.rs b/crates/atuin/src/command/client/stats.rs
index daffdd84..3ee60633 100644
--- a/crates/atuin/src/command/client/stats.rs
+++ b/crates/atuin/src/command/client/stats.rs
@@ -11,6 +11,18 @@ use atuin_client::{
 
 use atuin_history::stats::{compute, pretty_print};
 
+fn parse_ngram_size(s: &str) -> Result<usize, String> {
+    let value = s
+        .parse::<usize>()
+        .map_err(|_| format!("'{s}' is not a valid window size"))?;
+
+    if value == 0 {
+        return Err("ngram window size must be at least 1".to_string());
+    }
+
+    Ok(value)
+}
+
 #[derive(Parser, Debug)]
 #[command(infer_subcommands = true)]
 pub struct Cmd {
@@ -22,7 +34,7 @@ pub struct Cmd {
     count: usize,
 
     /// The number of consecutive commands to consider
-    #[arg(long, short, default_value = "1")]
+    #[arg(long, short, default_value = "1", value_parser = parse_ngram_size)]
     ngram_size: usize,
 }
 
-- 
cgit v1.3.1