From 273c7fd3d415aca1de2288d934d61c752f45bbf3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=B5=90=E5=8F=8B?= Date: Fri, 12 Jun 2026 17:58:59 +0900 Subject: [PATCH 1/9] =?UTF-8?q?feat:=20RTT=20=E7=B5=90=E6=9E=9C=E3=83=90?= =?UTF-8?q?=E3=83=BC=E3=81=AB=20log=20=E3=82=B9=E3=82=B1=E3=83=BC=E3=83=AB?= =?UTF-8?q?=E3=83=A2=E3=83=BC=E3=83=89=E3=82=92=E8=BF=BD=E5=8A=A0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit l キーで log 係数を off→e→e² と巡回し、log モードでは ↑/↓ の scale 値を 対数窓の floor(左端)として使う。scale を int→float64 化してサブミリ秒 スケール(0.01ms〜)に対応し、↑/↓ のラダーを sub-ms まで拡張した。 - monitor: Glyph/rttGlyph に logK 引数を追加。段=floor(ln(rtt/floor)/logK) を [0,7] にクランプし、linear と同じ左閉・半開の境界規約にそろえる。境界 の浮動小数落ちは index 空間の epsilon で補正し、float ドメインで clamp して から int 変換(gosec G115 を nolint なしの実ガードで解消) - config/cli: scale ディレクティブと -s/--scale を float64 化(小数受理、 lenient 維持+Inf ガード) - tui: l キーで logK 巡回・凡例に (l)og[lin/×e/×e²]・リロードで logK 保持 - README: -s 小数可・l キー・log モードを利用者契約として記載 linear(logK=0)の挙動は完全保存。make fmt/vet/lint/test/test-race 全通過。 --- README.md | 17 +++-- cmd/deadman/main.go | 8 +- cmd/deadman/main_test.go | 21 +++++- internal/config/config.go | 7 +- internal/config/config_test.go | 19 ++++- internal/config/prerelease_test.go | 2 +- internal/monitor/target.go | 61 ++++++++++++--- internal/monitor/target_test.go | 117 ++++++++++++++++++++++++++++- internal/tui/model.go | 66 ++++++++++------ internal/tui/view.go | 20 ++++- internal/tui/view_test.go | 30 +++++--- 11 files changed, 299 insertions(+), 69 deletions(-) diff --git a/README.md b/README.md index 98d05c4..1341a98 100644 --- a/README.md +++ b/README.md @@ -143,7 +143,7 @@ ssh 中継の例 `google-via-ssh 173.194.117.176 relay=X.X.X.X os=Linux` は、 ## オプション ```text --s, --scale N RTT バーグラフのスケール(ms 単位、既定 10。実行中は ↑/↓ でも変更可) +-s, --scale N RTT バーグラフのスケール(ms 単位、既定 10、小数可。実行中は ↑/↓ でも変更可) -a, --async-mode 全対象へ並列に ping を送る -b, --blink-arrow async モードで矢印を点滅させる -l, --logging DIR DIR 配下に対象ごとのログファイルを書き出す @@ -155,7 +155,8 @@ ssh 中継の例 `google-via-ssh 173.194.117.176 relay=X.X.X.X os=Linux` は、 ## 操作 ```text -↑ / ↓ RTT バーのスケールを段階的に変える(1→2→5→10→20→50→100 ms。既存のバーも再描画) +↑ / ↓ RTT バーのスケールを段階的に変える(0.01…0.1…1…10…100 ms。既存のバーも再描画) +l RTT スケールの対数表示を切り替える(linear → 底 e → 底 e²。↑/↓ の値が対数窓の左端=floor になる) p 統計値の表示精度を切り替える(ms → ms.1 → ms.2 → ms.3。常に ms 単位、小数桁数を増減) r 全対象の統計をリセットする(プログラムは動作したまま) R 設定ファイルを再読み込みする(Windows。Unix では SIGHUP を使用) @@ -191,7 +192,7 @@ split 2 一覧を 2 列で表示する(既定 1。明示した -c 列をより多く詰めたいときは、`m` / `v` / `p` で統計列を隠すと 1 列あたりの幅が縮み、より多くの列が入ります。 設定ファイルの**再読み込み**(Unix では SIGHUP、Windows では `R` キー)では、既存のエントリは統計・履歴を引き継ぎます(名前・アドレスおよび中継属性で同一性を判定)。 -`columns` の表示は設定ファイルの内容に戻りますが、`scale`・`precision`・`split`(段組み列数)は**実行中の値が保持されます**(`-s`/`-c` で起動した値やキーで調整した状態を、対象を入れ替えるだけの再読み込みで失わないため)。 +`columns` の表示は設定ファイルの内容に戻りますが、`scale`(log モードの状態を含む)・`precision`・`split`(段組み列数)は**実行中の値が保持されます**(`-s`/`-c` で起動した値やキーで調整した状態を、対象を入れ替えるだけの再読み込みで失わないため)。 端末のリサイズには自動で追随します。 ## 列(カラム)の表示 @@ -217,19 +218,23 @@ columns ADDRESS=off MIN=off MAX=off VIA=on RTT バーグラフの**スケール**と統計値の**表示精度**は、実行中にキーで切り替えられるほか、設定ファイルのディレクティブで起動時の既定を指定できます。 ```text -scale 5 RTT バー 1 段あたりの ms(↑/↓ キーと同じ。既定 10) +scale 5 RTT バー 1 段あたりの ms(↑/↓ キーと同じ。既定 10、小数可 例 scale 0.5) precision ms.1 統計値の表示精度(ms / ms.1 / ms.2 / ms.3 のいずれか。既定 ms) ``` - `scale` … RTT バーのグリフ(`▁▂▃▄▅▆▇█`)1 段が表す ms 幅です。 高速な LAN ではバーが潰れて差が見えないため、小さくすると見分けやすくなります。 - 実行中は `↑`(粗く)/ `↓`(細かく)で `1→2→5→10→20→50→100 ms` の段階を移動でき、**画面上の既存のバーも即座に再描画されます**。 + 実行中は `↑`(粗く)/ `↓`(細かく)で `0.01〜100 ms`(サブミリ秒を含む)の段階を移動でき、**画面上の既存のバーも即座に再描画されます**。 +- `log` モード … `l` キーで RTT スケールの**対数表示**を切り替えます(linear → 底 e → 底 e²)。 + 高速 LAN の低 RTT が最下段に潰れるのを避けつつ、桁をまたぐ広いレンジを 1 本のバーに収めたいときに使います。 + 対数モードでは `↑`/`↓` で設定するスケール値が**対数窓の左端(floor)**として働き、係数(底)で窓の広さが決まります(底 e で 7 段が約 2.6 桁、底 e² で約 5.2 桁)。 + 起動時は linear(オフ)で、CLI / 設定ファイルからの初期指定はありません(実行中に `l` で切り替え)。 - `precision` … 統計列(RTT / AVG / MIN / MAX / JIT)の数値表記です。 表示単位は常に ms で、`ms`(整数)/ `ms.1`(小数 1 桁)/ `ms.2`(小数 2 桁)/ `ms.3`(小数 3 桁 = マイクロ秒相当)から選べます。 `.N` の桁数を増やすほどサブミリ秒の差が見えますが列幅も広がります。 実行中は `p` キーで循環します。 -`scale` はコマンドラインの `-s` / `--scale` と同じ設定で、**明示した `-s` が `scale` ディレクティブより優先**されます(どちらも無ければ既定 10)。 +`scale` はコマンドラインの `-s` / `--scale` と同じ設定で、**明示した `-s` が `scale` ディレクティブより優先**されます(どちらも無ければ既定 10。小数値も指定でき、サブミリ秒のスケールに対応します)。 `precision` にコマンドラインフラグはありません。 再読み込みをまたいだ値の保持については「[操作](#操作)」を参照してください。 diff --git a/cmd/deadman/main.go b/cmd/deadman/main.go index 28687ed..aff9003 100644 --- a/cmd/deadman/main.go +++ b/cmd/deadman/main.go @@ -17,7 +17,7 @@ import ( ) // defaultScale is the default RTT bar gap in milliseconds. -const defaultScale = 10 +const defaultScale = 10.0 // version is the build version shown in the TUI title bar. It is overridden at build // time via -ldflags "-X main.version=..." (see the Makefile, which derives it from git @@ -29,7 +29,7 @@ var version = "dev" // -s flag defaults to 0, and a missing or invalid "scale" directive leaves cfg.Scale // at 0), so passing -s is never indistinguishable from its own default and a config // scale can take effect. -func resolveScale(cli, cfg int) int { +func resolveScale(cli, cfg float64) float64 { if cli > 0 { return cli } @@ -47,8 +47,8 @@ func resolveScale(cli, cfg int) int { // configfile intermix. func parseArgs(args []string) (tui.Options, error) { fs := flag.NewFlagSet("deadman", flag.ContinueOnError) - scale := fs.Int("s", 0, "scale of ping RTT bar gap (ms, default 10)") - fs.IntVar(scale, "scale", 0, "scale of ping RTT bar gap (ms, default 10)") + scale := fs.Float64("s", 0, "scale of ping RTT bar gap (ms, default 10, decimals allowed)") + fs.Float64Var(scale, "scale", 0, "scale of ping RTT bar gap (ms, default 10, decimals allowed)") async := fs.Bool("a", false, "send ping asynchronously") fs.BoolVar(async, "async-mode", false, "send ping asynchronously") blink := fs.Bool("b", false, "blink arrow in async mode") diff --git a/cmd/deadman/main_test.go b/cmd/deadman/main_test.go index 19283a8..2288bcc 100644 --- a/cmd/deadman/main_test.go +++ b/cmd/deadman/main_test.go @@ -73,7 +73,7 @@ func TestParseArgsScaleBlinkLog(t *testing.T) { } if opts.Scale != 20 { - t.Errorf("Scale = %d, want 20", opts.Scale) + t.Errorf("Scale = %g, want 20", opts.Scale) } if !opts.Blink { @@ -85,6 +85,18 @@ func TestParseArgsScaleBlinkLog(t *testing.T) { } } +func TestParseArgsScaleFractional(t *testing.T) { + // A fractional -s is accepted (sub-ms scale); the flag parses as float64. + opts, err := parseArgs([]string{"deadman.conf", "-s", "0.5"}) + if err != nil { + t.Fatal(err) + } + + if opts.Scale != 0.5 { + t.Errorf("Scale = %g, want 0.5", opts.Scale) + } +} + func TestParseArgsMissingConfig(t *testing.T) { _, err := parseArgs([]string{"-a"}) if err == nil { @@ -110,18 +122,19 @@ func TestParseArgsRejectsMultipleConfigs(t *testing.T) { func TestResolveScale(t *testing.T) { cases := []struct { name string - cli, cfg int - want int + cli, cfg float64 + want float64 }{ {"cli explicit wins over config", 20, 5, 20}, {"config used when cli unset", 0, 5, 5}, {"cli used when config unset", 7, 0, 7}, + {"sub-ms cli is honored", 0.5, 0, 0.5}, {"default when both unset", 0, 0, defaultScale}, } for _, c := range cases { t.Run(c.name, func(t *testing.T) { if got := resolveScale(c.cli, c.cfg); got != c.want { - t.Errorf("resolveScale(%d, %d) = %d, want %d", c.cli, c.cfg, got, c.want) + t.Errorf("resolveScale(%g, %g) = %g, want %g", c.cli, c.cfg, got, c.want) } }) } diff --git a/internal/config/config.go b/internal/config/config.go index 83e0b88..123f095 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -10,6 +10,7 @@ package config import ( "bufio" "io" + "math" "regexp" "strconv" "strings" @@ -42,7 +43,7 @@ type TargetSpec struct { type Config struct { Targets []TargetSpec Columns map[string]bool // column key (upper-case) -> shown. - Scale int // RTT-bar ms-per-step from a "scale" directive; 0 = unset. + Scale float64 // RTT-bar ms-per-step from a "scale" directive; 0 = unset. Precision string // stat-precision label from a "precision" directive; "" = unset. Cols int // newspaper-column count from a "split" directive; 0 = unset. } @@ -73,8 +74,8 @@ var directives = map[string]func(cfg *Config, args []string){ return } - n, err := strconv.Atoi(args[0]) - if err == nil && n > 0 { + n, err := strconv.ParseFloat(args[0], 64) + if err == nil && n > 0 && !math.IsInf(n, 0) { cfg.Scale = n } }, diff --git a/internal/config/config_test.go b/internal/config/config_test.go index f5d9df2..f60e466 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -218,7 +218,7 @@ func TestParseConfigDirectiveCaseInsensitive(t *testing.T) { } if cfg.Scale != 5 { - t.Errorf("Scale = %d, want 5", cfg.Scale) + t.Errorf("Scale = %g, want 5", cfg.Scale) } if cfg.Cols != 2 { @@ -403,7 +403,7 @@ func TestParseConfigScaleAndPrecision(t *testing.T) { } if cfg.Scale != 5 { - t.Errorf("Scale = %d, want 5", cfg.Scale) + t.Errorf("Scale = %g, want 5", cfg.Scale) } // The precision label is stored verbatim; the TUI validates it against its mode @@ -458,11 +458,24 @@ func TestParseConfigScaleLenient(t *testing.T) { } if cfg.Scale != 0 { - t.Errorf("ParseConfig(%q): Scale = %d, want 0 (unset)", in, cfg.Scale) + t.Errorf("ParseConfig(%q): Scale = %g, want 0 (unset)", in, cfg.Scale) } } } +func TestParseConfigScaleFractional(t *testing.T) { + // A fractional scale is accepted (sub-ms resolution); the value is stored verbatim + // as a float so the result bar can bucket sub-millisecond RTTs. + cfg, err := ParseConfig(strings.NewReader("scale 0.5\nhost 1.2.3.4\n")) + if err != nil { + t.Fatal(err) + } + + if cfg.Scale != 0.5 { + t.Errorf("Scale = %g, want 0.5", cfg.Scale) + } +} + func TestParseConfigEmpty(t *testing.T) { cfg, err := ParseConfig(strings.NewReader("\n \n#only comments\n")) if err != nil { diff --git a/internal/config/prerelease_test.go b/internal/config/prerelease_test.go index 5d697a9..0bb9ddb 100644 --- a/internal/config/prerelease_test.go +++ b/internal/config/prerelease_test.go @@ -15,7 +15,7 @@ func TestParseConfigStripsBOM(t *testing.T) { } if cfg.Scale != 5 { - t.Errorf("Scale = %d, want 5 (BOM must not block the first-line directive)", cfg.Scale) + t.Errorf("Scale = %g, want 5 (BOM must not block the first-line directive)", cfg.Scale) } if len(cfg.Targets) != 1 { diff --git a/internal/monitor/target.go b/internal/monitor/target.go index ee4a41c..f92be15 100644 --- a/internal/monitor/target.go +++ b/internal/monitor/target.go @@ -4,6 +4,7 @@ package monitor import ( + "math" "slices" "strconv" "strings" @@ -226,14 +227,24 @@ func (t *Target) foldSuccessRTT(rtt float64) { t.prevRTT = rtt } -// rttBars are the block elements for ascending RTT buckets; bar i is used when -// RTT < scale*(i+1), and the full block "█" for anything above the last bucket. +// rttBars are the block elements for ascending RTT buckets. In linear mode bar i +// covers [scale*i, scale*(i+1)); in log mode it covers the geometric band +// [floor*aⁱ, floor*a^(i+1)) with a = e^logK and floor = scale. The full block "█" +// renders for anything at or above the last band in either mode. var rttBars = []string{"▁", "▂", "▃", "▄", "▅", "▆", "▇"} +// boundaryEpsilon nudges the log segment index up before truncation so a value +// landing exactly on a band boundary (floor*aⁱ) is not pushed into the band below +// by floating-point error (e.g. floor=0.3, rtt=floor*e² yields 1.9999999998 rather +// than 2.0). True band boundaries are a full 1.0 apart in index space, so this can +// never promote a genuine non-boundary value into the next band. +const boundaryEpsilon = 1e-9 + // Glyph maps a result to its result-bar character. Failures map to X/t/s; a -// success maps to a block element scaled by the RTT scale (ms per step). The TUI -// calls this at render time, so the bar re-buckets when the scale changes. -func Glyph(res ping.Result, scale int) string { +// success maps to a block element bucketed by rttGlyph (linear when logK is 0, +// logarithmic otherwise). The TUI calls this at render time, so the bar re-buckets +// when the scale or log factor changes. +func Glyph(res ping.Result, scale float64, logK int) string { switch res.Code { case ping.SSHTimeout: return "t" @@ -244,18 +255,24 @@ func Glyph(res ping.Result, scale int) string { return "X" } - return rttGlyph(res.RTT, scale) + return rttGlyph(res.RTT, scale, logK) default: // unknown code: treat as a plain failure. return "X" } } -// rttGlyph picks the block element for a successful probe's RTT. -func rttGlyph(rtt float64, scale int) string { - step := float64(scale) +// rttGlyph picks the block element for a successful probe's RTT. With logK == 0 it +// buckets linearly: bar i covers [scale*i, scale*(i+1)), and "█" sits above the +// last bucket. With logK > 0 it delegates to logIndexGlyph, which buckets on a +// logarithmic window whose floor is scale and whose factor is e^logK. +func rttGlyph(rtt, scale float64, logK int) string { + if logK > 0 { + return logIndexGlyph(rtt, scale, logK) + } + for i, bar := range rttBars { - if rtt < step*float64(i+1) { + if rtt < scale*float64(i+1) { return bar } } @@ -263,6 +280,30 @@ func rttGlyph(rtt float64, scale int) string { return "█" } +// logIndexGlyph buckets rtt on a logarithmic scale: the segment index is +// floor(ln(rtt/floor)/logK) = log_{e^logK}(rtt/floor), clamped to [0, len(rttBars)]. +// Band i covers [floor*aⁱ, floor*a^(i+1)) with a = e^logK, mirroring the linear +// half-open, left-closed inclusion; an index at or above len(rttBars) overflows to +// "█" and a sub-floor RTT clamps to the lowest bar. A degenerate floor or a +// non-positive RTT (where ln is undefined) also falls back to the lowest bar. +func logIndexGlyph(rtt, floor float64, logK int) string { + if floor <= 0 || rtt <= 0 { + return rttBars[0] + } + + raw := math.Log(rtt/floor)/float64(logK) + boundaryEpsilon + + switch { + case math.IsNaN(raw) || raw < 0: + return rttBars[0] + case raw >= float64(len(rttBars)): + return "█" + default: + // raw is in [0, len(rttBars)) here, so the truncation is a valid bar index. + return rttBars[int(raw)] + } +} + // IsFailGlyph reports whether a glyph represents a failure (rendered in red). func IsFailGlyph(g string) bool { return g == "X" || g == "t" || g == "s" diff --git a/internal/monitor/target_test.go b/internal/monitor/target_test.go index 4ed66b0..59c6043 100644 --- a/internal/monitor/target_test.go +++ b/internal/monitor/target_test.go @@ -26,7 +26,7 @@ func TestGlyph(t *testing.T) { } for _, c := range cases { t.Run(c.name, func(t *testing.T) { - if got := Glyph(c.res, scale); got != c.want { + if got := Glyph(c.res, scale, 0); got != c.want { t.Errorf("Glyph(%+v) = %q, want %q", c.res, got, c.want) } }) @@ -69,7 +69,7 @@ func TestConsume(t *testing.T) { t.Errorf("Jit = %v, want 1.25", tg.Jit) } // History is newest-first: the last result was RTT 30, which renders ▄ at scale 10. - if got := tg.Results(1); len(got) != 1 || Glyph(got[0], 10) != "▄" { + if got := tg.Results(1); len(got) != 1 || Glyph(got[0], 10, 0) != "▄" { t.Errorf("Results(1) rendered at scale 10 = %v, want [▄]", got) } @@ -179,11 +179,120 @@ func TestResultsRescale(t *testing.T) { res := tg.Results(1)[0] // RTT 15: at scale 10 it lands in the 2nd bucket (10..20 -> ▂); at scale 5 it is // in the 4th (15 == 5*3, < 5*4 -> ▄). - if got := Glyph(res, 10); got != "▂" { + if got := Glyph(res, 10, 0); got != "▂" { t.Errorf("Glyph(RTT 15, scale 10) = %q, want ▂", got) } - if got := Glyph(res, 5); got != "▄" { + if got := Glyph(res, 5, 0); got != "▄" { t.Errorf("Glyph(RTT 15, scale 5) = %q, want ▄", got) } } + +// TestRttGlyphLog verifies logarithmic bucketing (logK > 0): band i covers +// [floor*aⁱ, floor*a^(i+1)) with a = e^logK and floor = scale, so an RTT exactly on +// a band boundary (floor*e^(logK*i)) lands in band i (glyph rttBars[i]) and one past +// the last band overflows to "█". Boundary inputs are generated with math.Exp so the +// floating-point edges (which boundaryEpsilon repairs) are exercised. +func TestRttGlyphLog(t *testing.T) { + exp := math.Exp + + cases := []struct { + name string + rtt float64 + scale float64 + logK int + want string + }{ + // logK=1 (base e), floor 1.0 — exact band boundaries. + {"k1_below_floor", 0.5, 1.0, 1, "▁"}, + {"k1_at_floor", 1.0, 1.0, 1, "▁"}, + {"k1_e1", exp(1), 1.0, 1, "▂"}, + {"k1_e2", exp(2), 1.0, 1, "▃"}, + {"k1_e3", exp(3), 1.0, 1, "▄"}, + {"k1_e4", exp(4), 1.0, 1, "▅"}, + {"k1_e5", exp(5), 1.0, 1, "▆"}, + {"k1_e6", exp(6), 1.0, 1, "▇"}, + {"k1_e7", exp(7), 1.0, 1, "█"}, + {"k1_huge", 1e9, 1.0, 1, "█"}, + // Mid-band (not on a boundary) lands in the lower band: floor(2.5) = 2. + {"k1_mid_2_3", exp(2.5), 1.0, 1, "▃"}, + + // logK=2 (base e²), floor 1.0. + {"k2_at_floor", 1.0, 1.0, 2, "▁"}, + {"k2_e2", exp(2), 1.0, 2, "▂"}, + {"k2_e4", exp(4), 1.0, 2, "▃"}, + {"k2_e6", exp(6), 1.0, 2, "▄"}, + {"k2_e8", exp(8), 1.0, 2, "▅"}, + {"k2_e10", exp(10), 1.0, 2, "▆"}, + {"k2_e12", exp(12), 1.0, 2, "▇"}, + {"k2_e14", exp(14), 1.0, 2, "█"}, + + // Non-unit floors: the exact-boundary inputs the epsilon guard repairs. + {"floor03_e2", 0.3 * exp(2), 0.3, 1, "▃"}, + {"floor25_e4", 2.5 * exp(4), 2.5, 1, "▅"}, + {"subms_floor_e3", 0.001 * exp(3), 0.001, 1, "▄"}, + + // Guards: zero/negative RTT and a degenerate floor fall back to ▁. + {"zero_rtt", 0.0, 1.0, 1, "▁"}, + {"neg_floor", exp(3), -1.0, 1, "▁"}, + + // logK=0 stays linear even through this table. + {"k0_linear_9_scale10", 9.0, 10.0, 0, "▁"}, + {"k0_linear_70_scale10", 70.0, 10.0, 0, "█"}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + res := ping.Result{Success: true, Code: ping.Success, RTT: c.rtt} + if got := Glyph(res, c.scale, c.logK); got != c.want { + t.Errorf("Glyph(RTT %v, scale %v, logK %d) = %q, want %q", + c.rtt, c.scale, c.logK, got, c.want) + } + }) + } +} + +// TestRttGlyphLogBoundaryStability is the regression guard for boundaryEpsilon: +// across non-unit floors and both factors, an RTT exactly on band boundary i +// (floor*e^(logK*i)) must land in band i (rttBars[i]) for i in 0..len(rttBars)-1 and +// overflow to "█" at i == len(rttBars). Without the epsilon nudge, exact boundaries +// such as floor=0.3,i=2 and floor=2.5,i=4 truncate one band too low. +func TestRttGlyphLogBoundaryStability(t *testing.T) { + floors := []float64{0.3, 1.0, 2.5, 7.0, 0.001} + for _, floor := range floors { + for _, logK := range []int{1, 2} { + for i := 0; i <= len(rttBars); i++ { + rtt := floor * math.Exp(float64(logK*i)) + + want := "█" + if i < len(rttBars) { + want = rttBars[i] + } + + res := ping.Result{Success: true, Code: ping.Success, RTT: rtt} + if got := Glyph(res, floor, logK); got != want { + t.Errorf("floor=%v logK=%d band=%d (rtt=%v): Glyph = %q, want %q", + floor, logK, i, rtt, got, want) + } + } + } + } +} + +// TestGlyphFailureCodesLogMode confirms log mode does not disturb the failure-code +// mapping: a failed probe is still X (and SSH timeout/failure t/s) whatever the log +// factor, since the switch routes before any RTT bucketing. +func TestGlyphFailureCodesLogMode(t *testing.T) { + cases := []struct { + res ping.Result + want string + }{ + {ping.Result{Code: ping.Failed}, "X"}, + {ping.Result{Code: ping.SSHTimeout}, "t"}, + {ping.Result{Code: ping.SSHFailed}, "s"}, + } + for _, c := range cases { + if got := Glyph(c.res, 1.0, 1); got != c.want { + t.Errorf("Glyph(%+v, logK=1) = %q, want %q", c.res, got, c.want) + } + } +} diff --git a/internal/tui/model.go b/internal/tui/model.go index 037eef0..00655a4 100644 --- a/internal/tui/model.go +++ b/internal/tui/model.go @@ -30,7 +30,7 @@ type Row struct { type Options struct { Async bool Blink bool - Scale int + Scale float64 Precision string // initial stat-precision label (config "precision"); "" = ms. LogDir string LogWriter *monitor.LogWriter // serializes -l log writes off the Update loop; nil = no logging. @@ -58,8 +58,9 @@ type Model struct { visible map[string]bool // per-column visibility (config defaults + 'm' toggle). - scale int // RTT-bar ms-per-step; adjusted live with up/down, applied when rendering glyphs. - precIdx int // index into precisionModes for the stat columns; cycled with 'p'. + scale float64 // RTT-bar ms-per-step; adjusted live with up/down, applied when rendering glyphs. + logK int // RTT-scale log factor: 0 = linear, 1 = base e, 2 = base e²; cycled with 'l'. + precIdx int // index into precisionModes for the stat columns; cycled with 'p'. scrollTop int // first visible row when the list exceeds the viewport; moved with j/k/g/G/PgUp/PgDn. @@ -268,9 +269,9 @@ func (m Model) handleKey(msg tea.KeyMsg) (tea.Model, tea.Cmd) { } // handleViewKey handles the display-only keys: the MIN/MAX ('m') and structural -// HOSTNAME/ADDRESS/VIA ('h'/'a'/'v') column toggles, the RTT-bar scale (up/down), -// the stat precision ('p'), and the newspaper-column count ('['/']'). An unknown -// key leaves the model unchanged. +// HOSTNAME/ADDRESS/VIA ('h'/'a'/'v') column toggles, the RTT-bar scale (up/down) and +// its log factor ('l'), the stat precision ('p'), and the newspaper-column count +// ('['/']'). An unknown key leaves the model unchanged. // // The layout-changing keys clampScroll after recalcWidths: a stat-column toggle // shifts minColumnWidth, which can change effectiveCols and thus the vertical @@ -291,13 +292,11 @@ func (m Model) handleViewKey(msg tea.KeyMsg) Model { // ('v'). Each one's width feeds the result-bar column and the multi-column // fit, so recompute the layout and re-pin the scroll, like the MIN/MAX toggle. return m.toggleStructuralCol(msg.String()).recalcWidths().clampScroll() - case "up": - // Coarser RTT-bar scale (more ms per step). Glyphs are bucketed at render - // time, so the existing bar re-buckets with no width change. - m.scale = scaleUp(m.scale) - case "down": - // Finer RTT-bar scale. - m.scale = scaleDown(m.scale) + case "up", "down", "l": + // RTT-bar scale (up/down step the ladder) and its log factor ('l' cycles + // 0->e->e²->0). Glyphs are bucketed at render time, so the bar re-buckets + // with no width change. + return m.adjustRTTScale(msg.String()) case "p": // Cycle the stat precision (ms -> ms.1 -> ms.2 -> ms.3); the column width // changes, so recompute the result-bar layout. @@ -320,6 +319,24 @@ func (m Model) handleViewKey(msg tea.KeyMsg) Model { return m } +// adjustRTTScale handles the RTT-bar scale and log-factor keys: up/down step the +// scale ladder and 'l' cycles the log factor (0 linear -> e -> e²). None of these +// changes a column width, so the caller needs no width recalc. +func (m Model) adjustRTTScale(key string) Model { + switch key { + case "up": + m.scale = scaleUp(m.scale) + case "down": + m.scale = scaleDown(m.scale) + case "l": + m.logK = (m.logK + 1) % logModes + default: + // Unreachable: handleViewKey routes only up/down/l here. + } + + return m +} + // toggleStructuralCol flips the visibility of the structural string column bound to // the key: 'h' -> HOSTNAME, 'a' -> ADDRESS, 'v' -> VIA. RESULT has no key and is // never hidden. The caller recomputes the layout afterwards, since each column's @@ -356,15 +373,20 @@ func (m Model) adjustCols(key string) Model { return m } -// scaleSteps is the ladder the up/down keys move the RTT-bar scale through (ms). -var scaleSteps = []int{1, 2, 5, 10, 20, 50, 100} +// logModes is the number of RTT-scale log factors the 'l' key cycles through: +// linear (0), base e (1), and base e² (2). +const logModes = 3 + +// scaleSteps is the ladder the up/down keys move the RTT-bar scale through (ms); it +// extends below 1ms so sub-millisecond LAN RTTs can be resolved. +var scaleSteps = []float64{0.01, 0.02, 0.05, 0.1, 0.2, 0.5, 1, 2, 5, 10, 20, 50, 100} // scaleUp returns the next coarser (larger-ms) rung above cur. An off-ladder cur // (e.g. from -s 7) snaps up to the nearest rung, so the live scale stays a free-form -// int while the keys move through sensible presets. At or above the top rung cur is +// value while the keys move through sensible presets. At or above the top rung cur is // preserved: Up means coarser, so it must never decrease the scale (a free-form // -s 1000 stays 1000 rather than snapping down to the ladder top). -func scaleUp(cur int) int { +func scaleUp(cur float64) float64 { for _, s := range scaleSteps { if s > cur { return s @@ -375,7 +397,7 @@ func scaleUp(cur int) int { } // scaleDown returns the next finer (smaller-ms) rung below cur, clamped at the bottom. -func scaleDown(cur int) int { +func scaleDown(cur float64) float64 { for _, s := range slices.Backward(scaleSteps) { if s < cur { return s @@ -386,10 +408,10 @@ func scaleDown(cur int) int { } // handleReload reparses the config and starts a fresh generation, so stale -// timers/results from the previous target set are ignored. The live scale/precision -// are intentionally preserved (not reset to config), unlike column visibility: -// scale has a CLI flag (-s) whose value a reload must not silently drop, and -// precision is a live, key-driven view setting. +// timers/results from the previous target set are ignored. The live +// scale/log-factor/precision are intentionally preserved (not reset to config), +// unlike column visibility: scale has a CLI flag (-s) whose value a reload must not +// silently drop, and the log factor and precision are live, key-driven view settings. func (m Model) handleReload() (tea.Model, tea.Cmd) { if r, ok := loadRows(m.opts.ConfigPath, m.rows); ok { m.rows = r.rows diff --git a/internal/tui/view.go b/internal/tui/view.go index 7ca2b49..7826316 100644 --- a/internal/tui/view.go +++ b/internal/tui/view.go @@ -81,7 +81,7 @@ func (m Model) View() string { // keysLine is the scale + key-legend line (line 2), factored into a method to sit // alongside the other fixed-line builders (centerTitle/titleLine/headerLine). func (m Model) keysLine() string { - s := rear + fmt.Sprintf("RTT Scale %dms.", m.scale) + s := rear + fmt.Sprintf("RTT Scale %gms.", m.scale) // The effective-vs-requested column count sits near the front, before the long // key legend, so the renderer's width truncation (it clips lines, not wraps) @@ -92,13 +92,27 @@ func (m Model) keysLine() string { } s += fmt.Sprintf( - " Keys: (q)uit (r)efresh (R)eload (m)in/max (v)ia (↑/↓)scale (p)recision[%s] ([/])cols", + " Keys: (q)uit (r)efresh (R)eload (m)in/max (v)ia (↑/↓)scale (l)og[%s] (p)recision[%s] ([/])cols", + logLabel(m.logK), m.precMode().Label, ) return s } +// logLabel renders the current RTT-scale log factor for the key legend: "lin" when +// linear (logK 0), "×e" for base e, and "×e²" for base e². +func logLabel(k int) string { + switch k { + case 1: + return "×e" + case 2: + return "×e²" + default: + return "lin" + } +} + // scrollStatus is the one-line position indicator shown below the row window when // the list is scrolled. It is truncated to the terminal width so it always // occupies exactly one physical line, which the row-window math assumes. @@ -215,7 +229,7 @@ func (m Model) targetLine(idx int, t *monitor.Target) string { var g strings.Builder for _, res := range t.Results(m.resW) { - ch := monitor.Glyph(res, m.scale) + ch := monitor.Glyph(res, m.scale, m.logK) if monitor.IsFailGlyph(ch) { g.WriteString(styleDown.Render(ch)) } else { diff --git a/internal/tui/view_test.go b/internal/tui/view_test.go index edef6f0..a46f761 100644 --- a/internal/tui/view_test.go +++ b/internal/tui/view_test.go @@ -593,13 +593,23 @@ func TestScaleStepKeys(t *testing.T) { t.Errorf("after up,up: want scale 20\n---\n%s", out) } - // down past the bottom rung clamps at 1ms. - _, out = drive(t, m, + // down past the bottom clamps at the sub-ms floor (0.01ms). From 20ms the ladder + // descends 20→10→5→2→1→0.5→0.2→0.1→0.05→0.02→0.01; press well past the bottom. + m, out = drive(t, m, + tea.KeyMsg{Type: tea.KeyDown}, tea.KeyMsg{Type: tea.KeyDown}, + tea.KeyMsg{Type: tea.KeyDown}, tea.KeyMsg{Type: tea.KeyDown}, + tea.KeyMsg{Type: tea.KeyDown}, tea.KeyMsg{Type: tea.KeyDown}, tea.KeyMsg{Type: tea.KeyDown}, tea.KeyMsg{Type: tea.KeyDown}, tea.KeyMsg{Type: tea.KeyDown}, tea.KeyMsg{Type: tea.KeyDown}, tea.KeyMsg{Type: tea.KeyDown}, tea.KeyMsg{Type: tea.KeyDown}) - if !strings.Contains(out, "RTT Scale 1ms") { - t.Errorf("down past the bottom should clamp at 1ms\n---\n%s", out) + if !strings.Contains(out, "RTT Scale 0.01ms") { + t.Errorf("down past the bottom should clamp at 0.01ms\n---\n%s", out) + } + + // further down past the floor stays clamped at 0.01ms. + _, out = drive(t, m, tea.KeyMsg{Type: tea.KeyDown}) + if !strings.Contains(out, "RTT Scale 0.01ms") { + t.Errorf("further down past the floor must stay at 0.01ms\n---\n%s", out) } } @@ -610,25 +620,27 @@ func TestScaleStepKeys(t *testing.T) { func TestScaleLadderBounds(t *testing.T) { cases := []struct { name string - cur int - wantUp, wantDn int + cur float64 + wantUp, wantDn float64 }{ {"within ladder", 10, 20, 5}, {"off-ladder below top", 7, 10, 5}, {"at top rung", 100, 100, 50}, {"above top rung stays put on up", 1000, 1000, 100}, // the Bug-1 regression guard. - {"at bottom rung", 1, 2, 1}, + {"at bottom rung", 0.01, 0.02, 0.01}, {"off-ladder near bottom", 3, 5, 2}, + {"sub-ms mid", 0.1, 0.2, 0.05}, + {"below bottom clamps", 0.005, 0.01, 0.01}, } for _, c := range cases { t.Run(c.name, func(t *testing.T) { if got := scaleUp(c.cur); got != c.wantUp { - t.Errorf("scaleUp(%d) = %d, want %d", c.cur, got, c.wantUp) + t.Errorf("scaleUp(%g) = %g, want %g", c.cur, got, c.wantUp) } if got := scaleDown(c.cur); got != c.wantDn { - t.Errorf("scaleDown(%d) = %d, want %d", c.cur, got, c.wantDn) + t.Errorf("scaleDown(%g) = %g, want %g", c.cur, got, c.wantDn) } }) } From 0c6929782b70bf9d0970f5c2254369c3dd66f7a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=B5=90=E5=8F=8B?= Date: Fri, 12 Jun 2026 18:55:41 +0900 Subject: [PATCH 2/9] =?UTF-8?q?fix:=20=E3=82=B3=E3=83=BC=E3=83=89=E3=83=AC?= =?UTF-8?q?=E3=83=93=E3=83=A5=E3=83=BC=E6=8C=87=E6=91=98=E5=AF=BE=E5=BF=9C?= =?UTF-8?q?=EF=BC=88=E7=B7=9A=E5=BD=A2=E5=A2=83=E7=95=8C=E8=A3=9C=E6=AD=A3?= =?UTF-8?q?=E3=83=BBscaleDown=20=E3=82=AF=E3=83=A9=E3=83=B3=E3=83=97?= =?UTF-8?q?=E3=83=BBCLI=20Inf=20=E3=82=AC=E3=83=BC=E3=83=89=E4=BB=96?= =?UTF-8?q?=EF=BC=89?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit マルチエージェント / codex レビューで挙がった実バグと契約の不備を修正。 実バグ: - 線形バケットに浮動小数の境界補正が無く、小数スケールで帯境界ちょうどの RTT が 1 段低く描画されていた(rttGlyph(0.3,0.1)→▃)。rttGlyph を linear/log 統一の step 計算にして boundaryEpsilon を両モードへ適用し、clamp を barForStep に集約。 線形境界テストを epsilon の実効回帰ガードにした - scaleDown が下限ラング未満で増加していた(-s 0.005 で ↓ が 0.01 に倍増)。 scaleUp の上端ガードの鏡像として cur を保持 - CLI の -s Inf が resolveScale を通過し全バーをフラットラインさせていた。config 側と同じ math.IsInf ガードを CLI 値にも適用 表示 / 契約: - log モードでヘッダを "RTT floor 10ms ×e" に変更(floor の意味を明示し、係数を 前方表示して 80 桁端末でのクリップを回避)。凡例は (l)og に簡略 - logModes 定数 + logLabel の switch を logFactors テーブル 1 本に統合 - README の対数窓の桁数を約 3.0/6.1 桁に訂正(7 段 = e^7/e^14) テスト追加: - 線形境界の epsilon 回帰ガード、l キー巡回・凡例・View 経路の logK 受け渡し・ リロード保持、config の scale inf/nan、resolveScale の非有限値 make fmt/vet/lint/test/test-race 全通過。 --- README.md | 3 +- cmd/deadman/main.go | 6 ++- cmd/deadman/main_test.go | 2 + internal/config/config_test.go | 10 +++-- internal/monitor/target.go | 60 ++++++++++++-------------- internal/monitor/target_test.go | 36 +++++++++++++--- internal/tui/model.go | 21 ++++++--- internal/tui/view.go | 28 ++++++------ internal/tui/view_test.go | 76 ++++++++++++++++++++++++++++++--- 9 files changed, 175 insertions(+), 67 deletions(-) diff --git a/README.md b/README.md index 1341a98..8a8b6a0 100644 --- a/README.md +++ b/README.md @@ -227,7 +227,8 @@ precision ms.1 統計値の表示精度(ms / ms.1 / ms.2 / ms.3 のいず 実行中は `↑`(粗く)/ `↓`(細かく)で `0.01〜100 ms`(サブミリ秒を含む)の段階を移動でき、**画面上の既存のバーも即座に再描画されます**。 - `log` モード … `l` キーで RTT スケールの**対数表示**を切り替えます(linear → 底 e → 底 e²)。 高速 LAN の低 RTT が最下段に潰れるのを避けつつ、桁をまたぐ広いレンジを 1 本のバーに収めたいときに使います。 - 対数モードでは `↑`/`↓` で設定するスケール値が**対数窓の左端(floor)**として働き、係数(底)で窓の広さが決まります(底 e で 7 段が約 2.6 桁、底 e² で約 5.2 桁)。 + 対数モードでは `↑`/`↓` で設定するスケール値が**対数窓の左端(floor)**として働き、係数(底)で窓の広さが決まります(底 e で 7 段が約 3.0 桁、底 e² で約 6.1 桁)。 + 対数モード中はヘッダ表示が `RTT floor 10ms ×e` のように floor と係数の表記に変わります。 起動時は linear(オフ)で、CLI / 設定ファイルからの初期指定はありません(実行中に `l` で切り替え)。 - `precision` … 統計列(RTT / AVG / MIN / MAX / JIT)の数値表記です。 表示単位は常に ms で、`ms`(整数)/ `ms.1`(小数 1 桁)/ `ms.2`(小数 2 桁)/ `ms.3`(小数 3 桁 = マイクロ秒相当)から選べます。 diff --git a/cmd/deadman/main.go b/cmd/deadman/main.go index aff9003..5c67f84 100644 --- a/cmd/deadman/main.go +++ b/cmd/deadman/main.go @@ -7,6 +7,7 @@ import ( "errors" "flag" "fmt" + "math" "os" tea "github.com/charmbracelet/bubbletea" @@ -30,7 +31,10 @@ var version = "dev" // at 0), so passing -s is never indistinguishable from its own default and a config // scale can take effect. func resolveScale(cli, cfg float64) float64 { - if cli > 0 { + // A non-finite CLI value (e.g. -s Inf, which flag.Float64 accepts) would flatten + // every bar, so reject it here just as the config "scale" directive rejects it; the + // fallback to a config scale or the default then applies. + if cli > 0 && !math.IsInf(cli, 0) { return cli } diff --git a/cmd/deadman/main_test.go b/cmd/deadman/main_test.go index 2288bcc..c13cc15 100644 --- a/cmd/deadman/main_test.go +++ b/cmd/deadman/main_test.go @@ -3,6 +3,7 @@ package main import ( "errors" "flag" + "math" "os" "testing" ) @@ -129,6 +130,7 @@ func TestResolveScale(t *testing.T) { {"config used when cli unset", 0, 5, 5}, {"cli used when config unset", 7, 0, 7}, {"sub-ms cli is honored", 0.5, 0, 0.5}, + {"non-finite cli falls back to config", math.Inf(1), 5, 5}, {"default when both unset", 0, 0, defaultScale}, } for _, c := range cases { diff --git a/internal/config/config_test.go b/internal/config/config_test.go index f60e466..3ba29ea 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -449,9 +449,13 @@ func TestParseConfigSplitLenient(t *testing.T) { } func TestParseConfigScaleLenient(t *testing.T) { - // A non-numeric or non-positive scale is ignored, leaving Scale unset (0) so the - // caller falls back to the CLI/default rather than aborting the parse. - for _, in := range []string{"scale abc\n", "scale -3\n", "scale 0\n", "scale\n"} { + // A non-numeric, non-positive, or non-finite scale is ignored, leaving Scale unset + // (0) so the caller falls back to the CLI/default rather than aborting the parse. + // "scale inf" is the case the math.IsInf guard catches (ParseFloat accepts it and + // +Inf > 0); "scale nan" is caught by the n > 0 check (NaN > 0 is false). + for _, in := range []string{ + "scale abc\n", "scale -3\n", "scale 0\n", "scale\n", "scale inf\n", "scale nan\n", + } { cfg, err := ParseConfig(strings.NewReader(in)) if err != nil { t.Fatalf("ParseConfig(%q) error: %v", in, err) diff --git a/internal/monitor/target.go b/internal/monitor/target.go index f92be15..dd60571 100644 --- a/internal/monitor/target.go +++ b/internal/monitor/target.go @@ -233,11 +233,12 @@ func (t *Target) foldSuccessRTT(rtt float64) { // renders for anything at or above the last band in either mode. var rttBars = []string{"▁", "▂", "▃", "▄", "▅", "▆", "▇"} -// boundaryEpsilon nudges the log segment index up before truncation so a value -// landing exactly on a band boundary (floor*aⁱ) is not pushed into the band below -// by floating-point error (e.g. floor=0.3, rtt=floor*e² yields 1.9999999998 rather -// than 2.0). True band boundaries are a full 1.0 apart in index space, so this can -// never promote a genuine non-boundary value into the next band. +// boundaryEpsilon nudges the bucket index up before truncation so an RTT exactly on a +// band boundary — where the true step value is a whole number that float rounding can +// render a hair below it (rtt/scale for rtt=0.3, scale=0.1 is 2.9999999999999996, not +// 3.0) — lands in the band it opens rather than the one below. The trade-off is +// deliberate: an RTT within ~1e-9 in step space just under a boundary rounds up too, a +// sliver far below display resolution. const boundaryEpsilon = 1e-9 // Glyph maps a result to its result-bar character. Failures map to X/t/s; a @@ -262,45 +263,38 @@ func Glyph(res ping.Result, scale float64, logK int) string { } } -// rttGlyph picks the block element for a successful probe's RTT. With logK == 0 it -// buckets linearly: bar i covers [scale*i, scale*(i+1)), and "█" sits above the -// last bucket. With logK > 0 it delegates to logIndexGlyph, which buckets on a -// logarithmic window whose floor is scale and whose factor is e^logK. +// rttGlyph picks the block element for a successful probe's RTT. The bucket index is +// computed in "step space" — rtt/scale for the linear scale (logK == 0) or +// log_{e^logK}(rtt/scale) for the logarithmic scale — then nudged by boundaryEpsilon +// and clamped by barForStep. Both regimes share the half-open, left-closed band +// inclusion: band i covers [scale*i, scale*(i+1)) linearly, [scale*aⁱ, scale*a^(i+1)) +// logarithmically (a = e^logK). A degenerate scale or a non-positive RTT (where the +// ratio/log is undefined) falls back to the lowest bar. func rttGlyph(rtt, scale float64, logK int) string { - if logK > 0 { - return logIndexGlyph(rtt, scale, logK) + if scale <= 0 || rtt <= 0 { + return rttBars[0] } - for i, bar := range rttBars { - if rtt < scale*float64(i+1) { - return bar - } + step := rtt / scale + if logK > 0 { + step = math.Log(step) / float64(logK) } - return "█" + return barForStep(step + boundaryEpsilon) } -// logIndexGlyph buckets rtt on a logarithmic scale: the segment index is -// floor(ln(rtt/floor)/logK) = log_{e^logK}(rtt/floor), clamped to [0, len(rttBars)]. -// Band i covers [floor*aⁱ, floor*a^(i+1)) with a = e^logK, mirroring the linear -// half-open, left-closed inclusion; an index at or above len(rttBars) overflows to -// "█" and a sub-floor RTT clamps to the lowest bar. A degenerate floor or a -// non-positive RTT (where ln is undefined) also falls back to the lowest bar. -func logIndexGlyph(rtt, floor float64, logK int) string { - if floor <= 0 || rtt <= 0 { - return rttBars[0] - } - - raw := math.Log(rtt/floor)/float64(logK) + boundaryEpsilon - +// barForStep maps a bucket index (already nudged by boundaryEpsilon) to a bar glyph, +// clamping to [0, len(rttBars)]: a negative index (RTT below the first band) or NaN +// yields the lowest bar, and an index at or above len(rttBars) overflows to "█". +func barForStep(step float64) string { switch { - case math.IsNaN(raw) || raw < 0: + case math.IsNaN(step) || step < 0: return rttBars[0] - case raw >= float64(len(rttBars)): + case step >= float64(len(rttBars)): return "█" default: - // raw is in [0, len(rttBars)) here, so the truncation is a valid bar index. - return rttBars[int(raw)] + // step is in [0, len(rttBars)) here, so the truncation is a valid bar index. + return rttBars[int(step)] } } diff --git a/internal/monitor/target_test.go b/internal/monitor/target_test.go index 59c6043..f80cb53 100644 --- a/internal/monitor/target_test.go +++ b/internal/monitor/target_test.go @@ -188,6 +188,29 @@ func TestResultsRescale(t *testing.T) { } } +// TestRttGlyphLinearBoundary is the regression guard for boundaryEpsilon: with a +// fractional linear scale, an RTT exactly on a band boundary like rtt=0.3, scale=0.1 +// (where 0.1*3 rounds to 0.30000000000000004 and rtt/scale to 2.9999999999999996) +// must land in the band it opens (▄), not the one below (▃). Removing boundaryEpsilon +// fails these — unlike the log boundary tests, whose floors hit exact integer steps. +func TestRttGlyphLinearBoundary(t *testing.T) { + cases := []struct { + rtt, scale float64 + want string + }{ + {0.3, 0.1, "▄"}, // 0.1*3 = 0.30000000000000004; 0.3 opens band 3. + {0.7, 0.1, "█"}, // 0.1*7 = 0.7000000000000001; 0.7 overflows the last band. + {0.2, 0.1, "▃"}, // band 2, no boundary artifact. + {0.05, 0.05, "▂"}, + } + for _, c := range cases { + res := ping.Result{Success: true, Code: ping.Success, RTT: c.rtt} + if got := Glyph(res, c.scale, 0); got != c.want { + t.Errorf("Glyph(RTT %v, scale %v, linear) = %q, want %q", c.rtt, c.scale, got, c.want) + } + } +} + // TestRttGlyphLog verifies logarithmic bucketing (logK > 0): band i covers // [floor*aⁱ, floor*a^(i+1)) with a = e^logK and floor = scale, so an RTT exactly on // a band boundary (floor*e^(logK*i)) lands in band i (glyph rttBars[i]) and one past @@ -227,7 +250,7 @@ func TestRttGlyphLog(t *testing.T) { {"k2_e12", exp(12), 1.0, 2, "▇"}, {"k2_e14", exp(14), 1.0, 2, "█"}, - // Non-unit floors: the exact-boundary inputs the epsilon guard repairs. + // Non-unit floors on exact band boundaries (these land on integer steps). {"floor03_e2", 0.3 * exp(2), 0.3, 1, "▃"}, {"floor25_e4", 2.5 * exp(4), 2.5, 1, "▅"}, {"subms_floor_e3", 0.001 * exp(3), 0.001, 1, "▄"}, @@ -251,11 +274,12 @@ func TestRttGlyphLog(t *testing.T) { } } -// TestRttGlyphLogBoundaryStability is the regression guard for boundaryEpsilon: -// across non-unit floors and both factors, an RTT exactly on band boundary i -// (floor*e^(logK*i)) must land in band i (rttBars[i]) for i in 0..len(rttBars)-1 and -// overflow to "█" at i == len(rttBars). Without the epsilon nudge, exact boundaries -// such as floor=0.3,i=2 and floor=2.5,i=4 truncate one band too low. +// TestRttGlyphLogBoundaryStability checks the log boundary contract across non-unit +// floors and both factors: an RTT exactly on band boundary i (floor*e^(logK*i)) lands +// in band i (rttBars[i]) for i in 0..len(rttBars)-1 and overflows to "█" at +// i == len(rttBars). These particular floors land on exact integer steps (so the +// boundaryEpsilon regression itself is guarded by TestRttGlyphLinearBoundary, where +// float rounding actually bites), but they pin the band semantics. func TestRttGlyphLogBoundaryStability(t *testing.T) { floors := []float64{0.3, 1.0, 2.5, 7.0, 0.001} for _, floor := range floors { diff --git a/internal/tui/model.go b/internal/tui/model.go index 00655a4..e985734 100644 --- a/internal/tui/model.go +++ b/internal/tui/model.go @@ -329,7 +329,7 @@ func (m Model) adjustRTTScale(key string) Model { case "down": m.scale = scaleDown(m.scale) case "l": - m.logK = (m.logK + 1) % logModes + m.logK = (m.logK + 1) % len(logFactors) default: // Unreachable: handleViewKey routes only up/down/l here. } @@ -373,9 +373,15 @@ func (m Model) adjustCols(key string) Model { return m } -// logModes is the number of RTT-scale log factors the 'l' key cycles through: -// linear (0), base e (1), and base e² (2). -const logModes = 3 +// logFactors is the single source of the 'l'-key cycle order and the legend labels for +// the RTT-scale log factor: linear (no log), then base e and base e². Adding a factor +// (e³, …) is one entry here; len(logFactors) bounds the cycle and the slice index is +// the logK value. +var logFactors = []struct{ Label string }{ + {"lin"}, + {"×e"}, + {"×e²"}, +} // scaleSteps is the ladder the up/down keys move the RTT-bar scale through (ms); it // extends below 1ms so sub-millisecond LAN RTTs can be resolved. @@ -396,7 +402,10 @@ func scaleUp(cur float64) float64 { return cur } -// scaleDown returns the next finer (smaller-ms) rung below cur, clamped at the bottom. +// scaleDown returns the next finer (smaller-ms) rung below cur. At or below the bottom +// rung cur is preserved: Down means finer, so it must never increase the scale — the +// mirror of scaleUp's top guard, so a free-form -s 0.005 stays 0.005 rather than being +// snapped up to the ladder bottom. func scaleDown(cur float64) float64 { for _, s := range slices.Backward(scaleSteps) { if s < cur { @@ -404,7 +413,7 @@ func scaleDown(cur float64) float64 { } } - return scaleSteps[0] + return cur } // handleReload reparses the config and starts a fresh generation, so stale diff --git a/internal/tui/view.go b/internal/tui/view.go index 7826316..59cf1e1 100644 --- a/internal/tui/view.go +++ b/internal/tui/view.go @@ -81,7 +81,15 @@ func (m Model) View() string { // keysLine is the scale + key-legend line (line 2), factored into a method to sit // alongside the other fixed-line builders (centerTitle/titleLine/headerLine). func (m Model) keysLine() string { - s := rear + fmt.Sprintf("RTT Scale %gms.", m.scale) + // In log mode the scale value is the window floor (not ms-per-step) and the factor + // sets the width, so relabel and surface the factor up front (clipped last, like the + // cols count below). + var s string + if m.logK > 0 { + s = rear + fmt.Sprintf("RTT floor %gms %s.", m.scale, logLabel(m.logK)) + } else { + s = rear + fmt.Sprintf("RTT Scale %gms.", m.scale) + } // The effective-vs-requested column count sits near the front, before the long // key legend, so the renderer's width truncation (it clips lines, not wraps) @@ -92,25 +100,21 @@ func (m Model) keysLine() string { } s += fmt.Sprintf( - " Keys: (q)uit (r)efresh (R)eload (m)in/max (v)ia (↑/↓)scale (l)og[%s] (p)recision[%s] ([/])cols", - logLabel(m.logK), + " Keys: (q)uit (r)efresh (R)eload (m)in/max (v)ia (↑/↓)scale (l)og (p)recision[%s] ([/])cols", m.precMode().Label, ) return s } -// logLabel renders the current RTT-scale log factor for the key legend: "lin" when -// linear (logK 0), "×e" for base e, and "×e²" for base e². +// logLabel renders the RTT-scale log factor for logK as its short legend label +// ("lin", "×e", "×e²"), reading the single logFactors table. func logLabel(k int) string { - switch k { - case 1: - return "×e" - case 2: - return "×e²" - default: - return "lin" + if k < 0 || k >= len(logFactors) { + return logFactors[0].Label } + + return logFactors[k].Label } // scrollStatus is the one-line position indicator shown below the row window when diff --git a/internal/tui/view_test.go b/internal/tui/view_test.go index a46f761..9433a7c 100644 --- a/internal/tui/view_test.go +++ b/internal/tui/view_test.go @@ -630,7 +630,7 @@ func TestScaleLadderBounds(t *testing.T) { {"at bottom rung", 0.01, 0.02, 0.01}, {"off-ladder near bottom", 3, 5, 2}, {"sub-ms mid", 0.1, 0.2, 0.05}, - {"below bottom clamps", 0.005, 0.01, 0.01}, + {"below bottom holds", 0.005, 0.01, 0.005}, } for _, c := range cases { @@ -676,6 +676,71 @@ func TestScaleRebucketsExistingBar(t *testing.T) { } } +// TestLogModeKey cycles the 'l' key (linear -> base e -> base e² -> linear), asserting +// the footer relabels to floor mode and surfaces the factor up front (×e / ×e²) and +// wraps back. This is the regression guard for the 'l' wiring and logLabel. +func TestLogModeKey(t *testing.T) { + specs := []config.TargetSpec{{Name: "h", Addr: "1.2.3.4", Relay: map[string]string{}}} + + m, err := New(specs, Options{Scale: 10}) + if err != nil { + t.Fatal(err) + } + + m, out := drive(t, m, tea.WindowSizeMsg{Width: 200, Height: 40}) + if !strings.Contains(out, "RTT Scale 10ms") { + t.Errorf("linear start should show 'RTT Scale 10ms'\n---\n%s", out) + } + + // 'l' -> base e: the footer switches to floor wording and the ×e factor. + m, out = drive(t, m, tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'l'}}) + if !strings.Contains(out, "RTT floor 10ms ×e.") { + t.Errorf("after l: want 'RTT floor 10ms ×e'\n---\n%s", out) + } + + // 'l' -> base e². + m, out = drive(t, m, tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'l'}}) + if !strings.Contains(out, "RTT floor 10ms ×e².") { + t.Errorf("after l,l: want ×e²\n---\n%s", out) + } + + // 'l' wraps back to linear. + _, out = drive(t, m, tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'l'}}) + if !strings.Contains(out, "RTT Scale 10ms") { + t.Errorf("after l×3: should wrap back to linear\n---\n%s", out) + } +} + +// TestLogModeRebucketsBar confirms 'l' re-buckets the on-screen bar through the View +// path (targetLine passing logK to monitor.Glyph): the same stored RTT 50 renders ▆ on +// the linear scale 10 but ▂ in log ×e (ln(50/10)≈1.6, band 1). +func TestLogModeRebucketsBar(t *testing.T) { + specs := []config.TargetSpec{{Name: "h", Addr: "1.2.3.4", Relay: map[string]string{}}} + + m, err := New(specs, Options{Scale: 10}) + if err != nil { + t.Fatal(err) + } + + m, out := drive(t, m, + tea.WindowSizeMsg{Width: 200, Height: 40}, + pingResultMsg{ + idx: 0, + target: m.rows[0].Target, + res: ping.Result{Success: true, Code: ping.Success, RTT: 50}, + }, + ) + if !strings.Contains(out, "▆") { + t.Errorf("RTT 50 at linear scale 10 should render ▆\n---\n%s", out) + } + + // switch to log ×e: 50ms re-buckets from ▆ to ▂. + _, out = drive(t, m, tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'l'}}) + if !strings.Contains(out, "▂") || strings.Contains(out, "▆") { + t.Errorf("in log ×e, RTT 50 should re-bucket to ▂ not ▆\n---\n%s", out) + } +} + func TestPrecisionFromConfigAtStartup(t *testing.T) { specs := []config.TargetSpec{{Name: "h", Addr: "1.2.3.4", Relay: map[string]string{}}} @@ -717,19 +782,20 @@ func TestReloadPreservesScaleAndPrecision(t *testing.T) { t.Fatal(err) } - // Live: step the scale to 5 and cycle precision to ms.1. + // Live: step the scale to 5, cycle precision to ms.1, and switch to log mode. m, _ = drive(t, m, tea.WindowSizeMsg{Width: 120, Height: 40}, tea.KeyMsg{Type: tea.KeyDown}, tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'p'}}, + tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'l'}}, ) // Reload reparses the file: columns reset to it (MIN hidden), but the live - // scale/precision are preserved (the documented, intentional asymmetry). + // scale/precision/log-factor are preserved (the documented, intentional asymmetry). _, out := drive(t, m, reloadMsg{}) - if !strings.Contains(out, "RTT Scale 5ms") || !strings.Contains(out, "(p)recision[ms.1]") { - t.Errorf("reload should preserve the live scale/precision\n---\n%s", out) + if !strings.Contains(out, "RTT floor 5ms ×e") || !strings.Contains(out, "(p)recision[ms.1]") { + t.Errorf("reload should preserve the live scale/precision/log-factor\n---\n%s", out) } if strings.Contains(out, "MIN") { From f46ad2294f3815b1e2f0161e021ddf78d5f80b63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=B5=90=E5=8F=8B?= Date: Sat, 13 Jun 2026 07:55:11 +0900 Subject: [PATCH 3/9] =?UTF-8?q?fix:=20=E3=82=B3=E3=83=BC=E3=83=89=E3=83=AC?= =?UTF-8?q?=E3=83=93=E3=83=A5=E3=83=BC=E6=8C=87=E6=91=98=E5=AF=BE=E5=BF=9C?= =?UTF-8?q?(=E7=AC=AC2=E5=BC=BE)=20=E2=80=94=20=E5=87=A1=E4=BE=8B=E3=83=A9?= =?UTF-8?q?=E3=83=99=E3=83=AB=E3=81=AE=20ambiguous=20=E5=B9=85=E3=83=BBsca?= =?UTF-8?q?le=20=E6=A4=9C=E8=A8=BC=E9=9B=86=E7=B4=84?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit claude code review のマージブロッカーと低 severity 指摘に対応。 マージブロッカー: - log モードの係数ラベル ×e/×e²(U+00D7/U+00B2)は East Asian Ambiguous 幅で、 日本語端末では 2 セル描画されて keysLine がレンダラの計算幅とずれ、画面が崩れる。 ASCII の xe/xe2 に変更(README・テストも追従、終端 '.' で厳密照合) その他: - scale の妥当性判定(>0 かつ有限)を config.ValidScale に集約。scaleDirective・ resolveScale・tui.New の 3 箇所のドリフトを防止(main の math 依存を解消) - tui.New が Options.Scale を正規化(resolveScale を介さない呼び出しで退化値が 全バーを ▁ にするのを防ぐ、Cols と同様) - logFactors のコメントに「添字 == logK == e の指数」契約と ASCII 制約を明記 - scale フィールドの doc に log モードでの floor 転用を追記(3 ファイル) - TestResolveScale に NaN ケース、リロード保持テストを終端照合に強化 make fmt/vet/lint/test/test-race 全通過。 --- README.md | 2 +- cmd/deadman/main.go | 7 +++---- cmd/deadman/main_test.go | 1 + internal/config/config.go | 12 ++++++++++-- internal/tui/model.go | 33 ++++++++++++++++++++++++--------- internal/tui/view.go | 2 +- internal/tui/view_test.go | 15 ++++++++------- 7 files changed, 48 insertions(+), 24 deletions(-) diff --git a/README.md b/README.md index 8a8b6a0..ee94fed 100644 --- a/README.md +++ b/README.md @@ -228,7 +228,7 @@ precision ms.1 統計値の表示精度(ms / ms.1 / ms.2 / ms.3 のいず - `log` モード … `l` キーで RTT スケールの**対数表示**を切り替えます(linear → 底 e → 底 e²)。 高速 LAN の低 RTT が最下段に潰れるのを避けつつ、桁をまたぐ広いレンジを 1 本のバーに収めたいときに使います。 対数モードでは `↑`/`↓` で設定するスケール値が**対数窓の左端(floor)**として働き、係数(底)で窓の広さが決まります(底 e で 7 段が約 3.0 桁、底 e² で約 6.1 桁)。 - 対数モード中はヘッダ表示が `RTT floor 10ms ×e` のように floor と係数の表記に変わります。 + 対数モード中はヘッダ表示が `RTT floor 10ms xe` のように floor と係数の表記に変わります(係数ラベルは底 e が `xe`、底 e² が `xe2`)。 起動時は linear(オフ)で、CLI / 設定ファイルからの初期指定はありません(実行中に `l` で切り替え)。 - `precision` … 統計列(RTT / AVG / MIN / MAX / JIT)の数値表記です。 表示単位は常に ms で、`ms`(整数)/ `ms.1`(小数 1 桁)/ `ms.2`(小数 2 桁)/ `ms.3`(小数 3 桁 = マイクロ秒相当)から選べます。 diff --git a/cmd/deadman/main.go b/cmd/deadman/main.go index 5c67f84..ff03245 100644 --- a/cmd/deadman/main.go +++ b/cmd/deadman/main.go @@ -7,7 +7,6 @@ import ( "errors" "flag" "fmt" - "math" "os" tea "github.com/charmbracelet/bubbletea" @@ -32,9 +31,9 @@ var version = "dev" // scale can take effect. func resolveScale(cli, cfg float64) float64 { // A non-finite CLI value (e.g. -s Inf, which flag.Float64 accepts) would flatten - // every bar, so reject it here just as the config "scale" directive rejects it; the - // fallback to a config scale or the default then applies. - if cli > 0 && !math.IsInf(cli, 0) { + // every bar, so it is rejected by the same config.ValidScale predicate the "scale" + // directive uses; the fallback to a config scale or the default then applies. + if config.ValidScale(cli) { return cli } diff --git a/cmd/deadman/main_test.go b/cmd/deadman/main_test.go index c13cc15..79c8d4d 100644 --- a/cmd/deadman/main_test.go +++ b/cmd/deadman/main_test.go @@ -131,6 +131,7 @@ func TestResolveScale(t *testing.T) { {"cli used when config unset", 7, 0, 7}, {"sub-ms cli is honored", 0.5, 0, 0.5}, {"non-finite cli falls back to config", math.Inf(1), 5, 5}, + {"nan cli falls back to config", math.NaN(), 5, 5}, {"default when both unset", 0, 0, defaultScale}, } for _, c := range cases { diff --git a/internal/config/config.go b/internal/config/config.go index 123f095..0bdd629 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -43,7 +43,7 @@ type TargetSpec struct { type Config struct { Targets []TargetSpec Columns map[string]bool // column key (upper-case) -> shown. - Scale float64 // RTT-bar ms-per-step from a "scale" directive; 0 = unset. + Scale float64 // RTT-bar ms-per-step (or log-mode floor) from a "scale" directive; 0 = unset. Precision string // stat-precision label from a "precision" directive; "" = unset. Cols int // newspaper-column count from a "split" directive; 0 = unset. } @@ -75,7 +75,7 @@ var directives = map[string]func(cfg *Config, args []string){ } n, err := strconv.ParseFloat(args[0], 64) - if err == nil && n > 0 && !math.IsInf(n, 0) { + if err == nil && ValidScale(n) { cfg.Scale = n } }, @@ -96,6 +96,14 @@ var directives = map[string]func(cfg *Config, args []string){ }, } +// ValidScale reports whether v is a usable RTT-bar scale: strictly positive and finite. +// It is the single predicate shared by the "scale" directive, the CLI -s resolver +// (cmd/deadman) and the TUI's startup normalization, so inf/nan/non-positive rejection +// cannot drift between them. +func ValidScale(v float64) bool { + return v > 0 && !math.IsInf(v, 0) +} + var reSeparator = regexp.MustCompile(`^-+$`) // relayKeys is the set of attribute keys routed into a target's relay map. "netns" is diff --git a/internal/tui/model.go b/internal/tui/model.go index e985734..d321613 100644 --- a/internal/tui/model.go +++ b/internal/tui/model.go @@ -30,8 +30,8 @@ type Row struct { type Options struct { Async bool Blink bool - Scale float64 - Precision string // initial stat-precision label (config "precision"); "" = ms. + Scale float64 // RTT-bar ms-per-step (the window floor in log mode); 0 lets New fall back. + Precision string // initial stat-precision label (config "precision"); "" = ms. LogDir string LogWriter *monitor.LogWriter // serializes -l log writes off the Update loop; nil = no logging. ConfigPath string @@ -58,7 +58,7 @@ type Model struct { visible map[string]bool // per-column visibility (config defaults + 'm' toggle). - scale float64 // RTT-bar ms-per-step; adjusted live with up/down, applied when rendering glyphs. + scale float64 // RTT-bar ms-per-step (the window floor in log mode); adjusted live with up/down. logK int // RTT-scale log factor: 0 = linear, 1 = base e, 2 = base e²; cycled with 'l'. precIdx int // index into precisionModes for the stat columns; cycled with 'p'. @@ -79,16 +79,28 @@ type Model struct { warnings []string // startup warnings (e.g. rp_filter, IPv6 nexthop ignored). } +// fallbackScale is the RTT-bar scale New uses when Options.Scale is unset or degenerate +// (a caller bypassing resolveScale); it matches the CLI default so behavior is uniform. +const fallbackScale = 10 + // New builds the initial model from parsed specs and options. The error return is // retained for call-site stability; New currently never fails (a target whose config // cannot be built degrades to a permanent-failure row rather than aborting). func New(specs []config.TargetSpec, opts Options) (Model, error) { rows, buildWarns := buildRows(specs, nil) + // Normalize the scale like Cols: a caller bypassing resolveScale (a test or an + // embedding) may pass a zero/degenerate Options.Scale, which would otherwise make + // rttGlyph's scale<=0 guard flatten every bar to ▁. + scale := opts.Scale + if !config.ValidScale(scale) { + scale = fallbackScale + } + return Model{ rows: rows, opts: opts, - scale: opts.Scale, + scale: scale, precIdx: precisionIndex(opts.Precision), hostInfo: hostInfo(), visible: buildVisible(opts.Columns), @@ -374,13 +386,16 @@ func (m Model) adjustCols(key string) Model { } // logFactors is the single source of the 'l'-key cycle order and the legend labels for -// the RTT-scale log factor: linear (no log), then base e and base e². Adding a factor -// (e³, …) is one entry here; len(logFactors) bounds the cycle and the slice index is -// the logK value. +// the RTT-scale log factor: linear (no log), then base e and base e². The slice index +// IS the logK value handed to monitor.Glyph (the ln divisor, so bars bucket on +// e^index), so a new entry must preserve that index==exponent contract — a non-e^n +// factor (e.g. base 10) needs a matching rttGlyph change, not just a label. Labels stay +// ASCII (narrow): "×"/"²" are East-Asian-ambiguous and render 2 cells on CJK terminals, +// desyncing the keys-line width from the renderer's measure. var logFactors = []struct{ Label string }{ {"lin"}, - {"×e"}, - {"×e²"}, + {"xe"}, + {"xe2"}, } // scaleSteps is the ladder the up/down keys move the RTT-bar scale through (ms); it diff --git a/internal/tui/view.go b/internal/tui/view.go index 59cf1e1..9324b95 100644 --- a/internal/tui/view.go +++ b/internal/tui/view.go @@ -108,7 +108,7 @@ func (m Model) keysLine() string { } // logLabel renders the RTT-scale log factor for logK as its short legend label -// ("lin", "×e", "×e²"), reading the single logFactors table. +// ("lin", "xe", "xe2"), reading the single logFactors table. func logLabel(k int) string { if k < 0 || k >= len(logFactors) { return logFactors[0].Label diff --git a/internal/tui/view_test.go b/internal/tui/view_test.go index 9433a7c..885e6f5 100644 --- a/internal/tui/view_test.go +++ b/internal/tui/view_test.go @@ -692,16 +692,16 @@ func TestLogModeKey(t *testing.T) { t.Errorf("linear start should show 'RTT Scale 10ms'\n---\n%s", out) } - // 'l' -> base e: the footer switches to floor wording and the ×e factor. + // 'l' -> base e: the footer switches to floor wording and the xe factor. m, out = drive(t, m, tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'l'}}) - if !strings.Contains(out, "RTT floor 10ms ×e.") { - t.Errorf("after l: want 'RTT floor 10ms ×e'\n---\n%s", out) + if !strings.Contains(out, "RTT floor 10ms xe.") { + t.Errorf("after l: want 'RTT floor 10ms xe'\n---\n%s", out) } - // 'l' -> base e². + // 'l' -> base e²: the xe2 factor (the trailing '.' keeps this from matching xe). m, out = drive(t, m, tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'l'}}) - if !strings.Contains(out, "RTT floor 10ms ×e².") { - t.Errorf("after l,l: want ×e²\n---\n%s", out) + if !strings.Contains(out, "RTT floor 10ms xe2.") { + t.Errorf("after l,l: want xe2\n---\n%s", out) } // 'l' wraps back to linear. @@ -794,7 +794,8 @@ func TestReloadPreservesScaleAndPrecision(t *testing.T) { // scale/precision/log-factor are preserved (the documented, intentional asymmetry). _, out := drive(t, m, reloadMsg{}) - if !strings.Contains(out, "RTT floor 5ms ×e") || !strings.Contains(out, "(p)recision[ms.1]") { + // The trailing '.' makes this fail if a reload bug advances logK 1->2 (xe -> xe2). + if !strings.Contains(out, "RTT floor 5ms xe.") || !strings.Contains(out, "(p)recision[ms.1]") { t.Errorf("reload should preserve the live scale/precision/log-factor\n---\n%s", out) } From 77551414220e6223cde127c9192d00b68cb90476 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=B5=90=E5=8F=8B?= Date: Sat, 13 Jun 2026 08:31:55 +0900 Subject: [PATCH 4/9] go mod tidy --- go.sum | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/go.sum b/go.sum index 6013b40..37690ec 100644 --- a/go.sum +++ b/go.sum @@ -1,7 +1,5 @@ github.com/aymanbagabas/go-osc52/v2 v2.0.1 h1:HwpRHbFMcZLEVr42D4p7XBqjyuxQH5SMiErDT4WkJ2k= github.com/aymanbagabas/go-osc52/v2 v2.0.1/go.mod h1:uYgXzlJ7ZpABp8OJ+exZzJJhRNQ2ASbcXHWsFqH8hp8= -github.com/aymanbagabas/go-udiff v0.2.0/go.mod h1:RE4Ex0qsGkTAJoQdQQCA0uG+nAzJO/pI/QwceO5fgrA= -github.com/bits-and-blooms/bitset v1.24.4/go.mod h1:7hO7Gc7Pp1vODcmWvKMRA9BNmbv6a/7QIWpPxHddWR8= github.com/charmbracelet/bubbletea v1.3.10 h1:otUDHWMMzQSB0Pkc87rm691KZ3SWa4KUlvF9nRvCICw= github.com/charmbracelet/bubbletea v1.3.10/go.mod h1:ORQfo0fk8U+po9VaNvnV95UPWA1BitP1E0N6xJPlHr4= github.com/charmbracelet/colorprofile v0.4.3 h1:QPa1IWkYI+AOB+fE+mg/5/4HRMZcaXex9t5KX76i20Q= @@ -12,12 +10,10 @@ github.com/charmbracelet/x/ansi v0.11.7 h1:kzv1kJvjg2S3r9KHo8hDdHFQLEqn4RBCb39dA github.com/charmbracelet/x/ansi v0.11.7/go.mod h1:9qGpnAVYz+8ACONkZBUWPtL7lulP9No6p1epAihUZwQ= github.com/charmbracelet/x/cellbuf v0.0.15 h1:ur3pZy0o6z/R7EylET877CBxaiE1Sp1GMxoFPAIztPI= github.com/charmbracelet/x/cellbuf v0.0.15/go.mod h1:J1YVbR7MUuEGIFPCaaZ96KDl5NoS0DAWkskup+mOY+Q= -github.com/charmbracelet/x/exp/golden v0.0.0-20240806155701-69247e0abc2a/go.mod h1:wDlXFlCrmJ8J+swcL/MnGUuYnqgQdW9rhSD61oNMb6U= github.com/charmbracelet/x/term v0.2.2 h1:xVRT/S2ZcKdhhOuSP4t5cLi5o+JxklsoEObBSgfgZRk= github.com/charmbracelet/x/term v0.2.2/go.mod h1:kF8CY5RddLWrsgVwpw4kAa6TESp6EB5y3uxGLeCqzAI= github.com/clipperhouse/displaywidth v0.11.0 h1:lBc6kY44VFw+TDx4I8opi/EtL9m20WSEFgwIwO+UVM8= github.com/clipperhouse/displaywidth v0.11.0/go.mod h1:bkrFNkf81G8HyVqmKGxsPufD3JhNl3dSqnGhOoSD/o0= -github.com/clipperhouse/stringish v0.1.1/go.mod h1:v/WhFtE1q0ovMta2+m+UbpZ+2/HEXNWYXQgCt4hdOzA= github.com/clipperhouse/uax29/v2 v2.7.0 h1:+gs4oBZ2gPfVrKPthwbMzWZDaAFPGYK72F0NJv2v7Vk= github.com/clipperhouse/uax29/v2 v2.7.0/go.mod h1:EFJ2TJMRUaplDxHKj1qAEhCtQPW2tJSwu5BF98AuoVM= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= @@ -26,8 +22,6 @@ github.com/erikgeiser/coninput v0.0.0-20211004153227-1c3628e74d0f h1:Y/CXytFA4m6 github.com/erikgeiser/coninput v0.0.0-20211004153227-1c3628e74d0f/go.mod h1:vw97MGsxSvLiUE2X8qFplwetxpGLQrlU1Q9AUEIzCaM= github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= -github.com/jordanlewis/gcassert v0.0.0-20250430164644-389ef753e22e/go.mod h1:ZybsQk6DWyN5t7An1MuPm1gtSZ1xDaTXS9ZjIOxvQrk= -github.com/kr/pretty v0.3.1/go.mod h1:hoEshYVHaxMs3cyo3Yncou5ZscifuDolrwPKZanG3xk= github.com/lucasb-eyer/go-colorful v1.4.0 h1:UtrWVfLdarDgc44HcS7pYloGHJUjHV/4FwW4TvVgFr4= github.com/lucasb-eyer/go-colorful v1.4.0/go.mod h1:R4dSotOR9KMtayYi1e77YzuveK+i7ruzyGqttikkLy0= github.com/mattn/go-isatty v0.0.22 h1:j8l17JJ9i6VGPUFUYoTUKPSgKe/83EYU2zBC7YNKMw4= @@ -48,12 +42,10 @@ github.com/prometheus-community/pro-bing v0.9.0 h1:M/zt1lL7cmbK+wm40RPS4waaiAN2E github.com/prometheus-community/pro-bing v0.9.0/go.mod h1:IBeW2ScY7sAWv4mYjH0xDDigqfe7kXeVxNdbNKdBHWY= github.com/quic-go/go-ossfuzz-seeds v0.1.0 h1:APacT+iIaNF6fd8AGEiN3bT/Jtkd2jz4v4TzM7MFjy0= github.com/quic-go/go-ossfuzz-seeds v0.1.0/go.mod h1:3IOHRbJIc+L6YKMwfDtJAM9Vj9k0YY4muhuyUYk5tbk= -github.com/quic-go/qpack v0.6.0/go.mod h1:lUpLKChi8njB4ty2bFLX2x4gzDqXwUpaO1DP9qMDZII= github.com/quic-go/quic-go v0.60.0 h1:xcQioE8OM66UQLeUMHltK1CCcOu3JbVB4JAQdDQSB+0= github.com/quic-go/quic-go v0.60.0/go.mod h1:wpKpjmPpftl30sL6pFh7REVpjbcCVy4zt2vDyK1TuJk= github.com/rivo/uniseg v0.4.7 h1:WUdvkW8uEhrYfLC4ZzdpI2ztxP1I582+49Oc5Mq64VQ= github.com/rivo/uniseg v0.4.7/go.mod h1:FN3SvrM+Zdj16jyLfmOkMNblXMcoc8DfTHruCPUcx88= -github.com/rogpeppe/go-internal v1.10.0/go.mod h1:UQnix2H7Ngw/k4C5ijL5+65zddjncjaFoBhdsK/akog= github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U= github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e h1:JVG44RsyaB9T2KIHavMF/ppJZNG9ZpyihvCd0w101no= @@ -64,7 +56,6 @@ golang.org/x/crypto v0.53.0 h1:QZ4Muo8THX6CizN2vPPd5fBGHyogrdK9fG4wLPFUsto= golang.org/x/crypto v0.53.0/go.mod h1:DNLU434OwVakk9PzuwV8w62mAJpRJL3vsgcfp4Qnsio= golang.org/x/exp v0.0.0-20231006140011-7918f672742d h1:jtJma62tbqLibJ5sFQz8bKtEM8rJBtfilJ2qTU199MI= golang.org/x/exp v0.0.0-20231006140011-7918f672742d/go.mod h1:ldy0pHrwJyGW56pPQzzkH36rKxoZW1tw7ZJpeKx+hdo= -golang.org/x/mod v0.36.0/go.mod h1:moc6ELqsWcOw5Ef3xVprK5ul/MvtVvkIXLziUOICjUQ= golang.org/x/net v0.56.0 h1:Rw8j/hFzGvJUZwNBXnAtf5sVDVt+65SK2C7IxCxZt5o= golang.org/x/net v0.56.0/go.mod h1:D3Ku6r+V6JROoZK144D2XfMHFcMq/0zSfLelVTCFKec= golang.org/x/sync v0.21.0 h1:HLII4xRRTtCRkxYp4HNFF0Js/Og6q2i++KXbg0gHCwM= @@ -72,10 +63,7 @@ golang.org/x/sync v0.21.0/go.mod h1:9xrNwdLfx4jkKbNva9FpL6vEN7evnE43NNNJQ2LF3+0= golang.org/x/sys v0.0.0-20210809222454-d867a43fc93e/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.46.0 h1:noSf2Fq6F8DBgS+LysIkx7rIExoNHJsxOAtPp4rthXw= golang.org/x/sys v0.46.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= -golang.org/x/term v0.44.0/go.mod h1:7ze4MdzUzLXpSAoFP1H0bOI9aXDqveSvatT5vKcFh2Y= golang.org/x/text v0.38.0 h1:sXmwo9DwP3OK9EZ7PqAdaooSGozfl/3a6/xJcbzPRhE= golang.org/x/text v0.38.0/go.mod h1:YXZt3QhHUKYT53r2lLKFIVi6Ao1jdzrTR/KQ09qyxF4= -golang.org/x/tools v0.45.0/go.mod h1:LuUGqqaXcXMEFEruIVJVm5mgDD8vww/z/SR1gQ4uE/0= -gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c/go.mod h1:JHkPIbrfpd72SG/EVd6muEfDQjcINNoR0C8j2r3qZ4Q= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= From 5e554d9b10ab8fe16e3f32171db5a56311bec846 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=B5=90=E5=8F=8B?= Date: Sat, 13 Jun 2026 09:18:25 +0900 Subject: [PATCH 5/9] =?UTF-8?q?fix:=20=E3=82=B3=E3=83=BC=E3=83=89=E3=83=AC?= =?UTF-8?q?=E3=83=93=E3=83=A5=E3=83=BC=E6=8C=87=E6=91=98=E5=AF=BE=E5=BF=9C?= =?UTF-8?q?(=E7=AC=AC3=E5=BC=BE)=20=E2=80=94=20logFactors=20=E3=82=92?= =?UTF-8?q?=E3=83=87=E3=83=BC=E3=82=BF=E9=A7=86=E5=8B=95=E5=8C=96=E3=83=BB?= =?UTF-8?q?=E6=97=A2=E5=AE=9A=E5=80=A4/=E8=BF=B0=E8=AA=9E=E3=81=AE?= =?UTF-8?q?=E5=8D=98=E4=B8=80=E6=83=85=E5=A0=B1=E6=BA=90=E5=8C=96?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit /code-review ultra(xhigh) の確認済み指摘のうち correctness/cleanup/doc を対応。 意図的と裁定された 3 件(テスト重複・adjustRTTScale 二重ディスパッチ・log 起動指定の 非対称)は、いずれもレビュー自身が intentional/採用非推奨としたため不対応。 単一情報源化(上位3件を同方向で解消): - logFactors を named type logFactor{Label, LnBase} 化(precisionMode に倣う)。 スライス添字が ln 除数(logK)を兼ねる暗黙契約を解消し、Glyph には float64 の LnBase を 渡す。底10など非 e^n の係数も行追加だけで済む(rttGlyph 変更不要)。 併せて model の logK フィールドを logIdx に改名(添字であって除数でないことを明示)。 - 既定スケールを config.DefaultScale に集約。cmd の defaultScale=10.0 と tui の fallbackScale=10 の手動同期コピーを解消し、ValidScale の隣に置いて単一情報源化。 - resolveScale の cfg 側 `cfg > 0` を config.ValidScale(cfg) に統一。directive パーサを 経ない Config.Scale 生成元が現れても +Inf が素通りしない。 その他: - barForStep が boundaryEpsilon を自前で加算するよう変更。nudge→truncate プロトコルを 関数境界に跨らせず、将来の第二の呼び出し元が加算を忘れて off-by-one を再発させない。 - Glyph/rttGlyph の doc を `logK==0 で linear/それ以外 log` から `lnBase<=0 で linear/正で log` へ字句修正(負の値も無音で linear なのは仕様どおり)。 - keysLine の %g を strconv.FormatFloat('f',-1) に。明示指定 -s 1000000 の "1e+06ms" 指数表記を回避。はしご値(0.01/10/100)はバイト一致のまま。 テストも lnBase 語彙・config.DefaultScale 参照・cfg 非有限フォールバックに追従。 make check 全通過(tidy-check/fmt-check/vet/lint/test)・test-race 通過。go.sum 不変。 --- cmd/deadman/main.go | 22 ++++++++--------- cmd/deadman/main_test.go | 6 ++++- internal/config/config.go | 6 +++++ internal/monitor/target.go | 41 ++++++++++++++++-------------- internal/monitor/target_test.go | 42 +++++++++++++++---------------- internal/tui/model.go | 44 +++++++++++++++++++-------------- internal/tui/view.go | 34 ++++++++++++++++--------- internal/tui/view_test.go | 8 +++--- 8 files changed, 116 insertions(+), 87 deletions(-) diff --git a/cmd/deadman/main.go b/cmd/deadman/main.go index ff03245..160fc19 100644 --- a/cmd/deadman/main.go +++ b/cmd/deadman/main.go @@ -16,32 +16,30 @@ import ( "github.com/yuu61/deadman/internal/tui" ) -// defaultScale is the default RTT bar gap in milliseconds. -const defaultScale = 10.0 - // version is the build version shown in the TUI title bar. It is overridden at build // time via -ldflags "-X main.version=..." (see the Makefile, which derives it from git // describe); a plain `go install`/`go build` leaves it at "dev". var version = "dev" // resolveScale picks the effective RTT-bar scale: an explicit CLI -s wins, else a -// config "scale" directive, else defaultScale. 0 means "unset" for both inputs (the -// -s flag defaults to 0, and a missing or invalid "scale" directive leaves cfg.Scale -// at 0), so passing -s is never indistinguishable from its own default and a config -// scale can take effect. +// config "scale" directive, else config.DefaultScale. 0 means "unset" for both inputs +// (the -s flag defaults to 0, and a missing or invalid "scale" directive leaves +// cfg.Scale at 0), so passing -s is never indistinguishable from its own default and a +// config scale can take effect. func resolveScale(cli, cfg float64) float64 { - // A non-finite CLI value (e.g. -s Inf, which flag.Float64 accepts) would flatten - // every bar, so it is rejected by the same config.ValidScale predicate the "scale" - // directive uses; the fallback to a config scale or the default then applies. + // Both inputs go through the same config.ValidScale predicate the "scale" directive + // uses, so a non-finite value (e.g. -s Inf, which flag.Float64 accepts, or a cfg + // built outside the directive parser) is rejected rather than flattening every bar; + // the fallback to the next source then applies. if config.ValidScale(cli) { return cli } - if cfg > 0 { + if config.ValidScale(cfg) { return cfg } - return defaultScale + return config.DefaultScale } // parseArgs parses the command line into TUI options. Flags may appear before or diff --git a/cmd/deadman/main_test.go b/cmd/deadman/main_test.go index 79c8d4d..46d5c67 100644 --- a/cmd/deadman/main_test.go +++ b/cmd/deadman/main_test.go @@ -6,6 +6,8 @@ import ( "math" "os" "testing" + + "github.com/yuu61/deadman/internal/config" ) // -h/--help must surface flag.ErrHelp so main can exit 0 (success) rather than the @@ -132,7 +134,9 @@ func TestResolveScale(t *testing.T) { {"sub-ms cli is honored", 0.5, 0, 0.5}, {"non-finite cli falls back to config", math.Inf(1), 5, 5}, {"nan cli falls back to config", math.NaN(), 5, 5}, - {"default when both unset", 0, 0, defaultScale}, + {"non-finite cfg falls back to default", 0, math.Inf(1), config.DefaultScale}, + {"nan cfg falls back to default", 0, math.NaN(), config.DefaultScale}, + {"default when both unset", 0, 0, config.DefaultScale}, } for _, c := range cases { t.Run(c.name, func(t *testing.T) { diff --git a/internal/config/config.go b/internal/config/config.go index 0bdd629..dc1b8c6 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -104,6 +104,12 @@ func ValidScale(v float64) bool { return v > 0 && !math.IsInf(v, 0) } +// DefaultScale is the RTT-bar scale used when neither the CLI -s flag nor a "scale" +// directive supplies a valid value. It sits beside ValidScale so the CLI resolver +// (cmd/deadman) and the TUI's startup normalization share one default rather than each +// hardcoding it — the same single-source rationale as ValidScale. +const DefaultScale = 10.0 + var reSeparator = regexp.MustCompile(`^-+$`) // relayKeys is the set of attribute keys routed into a target's relay map. "netns" is diff --git a/internal/monitor/target.go b/internal/monitor/target.go index dd60571..62669e1 100644 --- a/internal/monitor/target.go +++ b/internal/monitor/target.go @@ -229,7 +229,7 @@ func (t *Target) foldSuccessRTT(rtt float64) { // rttBars are the block elements for ascending RTT buckets. In linear mode bar i // covers [scale*i, scale*(i+1)); in log mode it covers the geometric band -// [floor*aⁱ, floor*a^(i+1)) with a = e^logK and floor = scale. The full block "█" +// [floor*aⁱ, floor*a^(i+1)) with a = e^lnBase and floor = scale. The full block "█" // renders for anything at or above the last band in either mode. var rttBars = []string{"▁", "▂", "▃", "▄", "▅", "▆", "▇"} @@ -238,14 +238,15 @@ var rttBars = []string{"▁", "▂", "▃", "▄", "▅", "▆", "▇"} // render a hair below it (rtt/scale for rtt=0.3, scale=0.1 is 2.9999999999999996, not // 3.0) — lands in the band it opens rather than the one below. The trade-off is // deliberate: an RTT within ~1e-9 in step space just under a boundary rounds up too, a -// sliver far below display resolution. +// sliver far below display resolution. barForStep applies it, so every caller shares +// the nudge-then-truncate protocol without re-adding it. const boundaryEpsilon = 1e-9 // Glyph maps a result to its result-bar character. Failures map to X/t/s; a -// success maps to a block element bucketed by rttGlyph (linear when logK is 0, -// logarithmic otherwise). The TUI calls this at render time, so the bar re-buckets +// success maps to a block element bucketed by rttGlyph (logarithmic when lnBase is +// positive, linear otherwise). The TUI calls this at render time, so the bar re-buckets // when the scale or log factor changes. -func Glyph(res ping.Result, scale float64, logK int) string { +func Glyph(res ping.Result, scale, lnBase float64) string { switch res.Code { case ping.SSHTimeout: return "t" @@ -256,7 +257,7 @@ func Glyph(res ping.Result, scale float64, logK int) string { return "X" } - return rttGlyph(res.RTT, scale, logK) + return rttGlyph(res.RTT, scale, lnBase) default: // unknown code: treat as a plain failure. return "X" @@ -264,29 +265,31 @@ func Glyph(res ping.Result, scale float64, logK int) string { } // rttGlyph picks the block element for a successful probe's RTT. The bucket index is -// computed in "step space" — rtt/scale for the linear scale (logK == 0) or -// log_{e^logK}(rtt/scale) for the logarithmic scale — then nudged by boundaryEpsilon -// and clamped by barForStep. Both regimes share the half-open, left-closed band -// inclusion: band i covers [scale*i, scale*(i+1)) linearly, [scale*aⁱ, scale*a^(i+1)) -// logarithmically (a = e^logK). A degenerate scale or a non-positive RTT (where the -// ratio/log is undefined) falls back to the lowest bar. -func rttGlyph(rtt, scale float64, logK int) string { +// computed in "step space" — rtt/scale for the linear scale (lnBase <= 0) or +// ln(rtt/scale)/lnBase for the logarithmic scale (a base-e^lnBase log) — then clamped by +// barForStep. Both regimes share the half-open, left-closed band inclusion: band i +// covers [scale*i, scale*(i+1)) linearly, [scale*aⁱ, scale*a^(i+1)) logarithmically +// (a = e^lnBase). A degenerate scale or a non-positive RTT (where the ratio/log is +// undefined) falls back to the lowest bar. +func rttGlyph(rtt, scale, lnBase float64) string { if scale <= 0 || rtt <= 0 { return rttBars[0] } step := rtt / scale - if logK > 0 { - step = math.Log(step) / float64(logK) + if lnBase > 0 { + step = math.Log(step) / lnBase } - return barForStep(step + boundaryEpsilon) + return barForStep(step) } -// barForStep maps a bucket index (already nudged by boundaryEpsilon) to a bar glyph, -// clamping to [0, len(rttBars)]: a negative index (RTT below the first band) or NaN -// yields the lowest bar, and an index at or above len(rttBars) overflows to "█". +// barForStep maps a bucket index to a bar glyph, clamping to [0, len(rttBars)]: a +// negative index (RTT below the first band) or NaN yields the lowest bar, and an index +// at or above len(rttBars) overflows to "█". It applies boundaryEpsilon itself so the +// nudge-then-truncate protocol lives in one place rather than across the call boundary. func barForStep(step float64) string { + step += boundaryEpsilon switch { case math.IsNaN(step) || step < 0: return rttBars[0] diff --git a/internal/monitor/target_test.go b/internal/monitor/target_test.go index f80cb53..8542ae6 100644 --- a/internal/monitor/target_test.go +++ b/internal/monitor/target_test.go @@ -211,22 +211,22 @@ func TestRttGlyphLinearBoundary(t *testing.T) { } } -// TestRttGlyphLog verifies logarithmic bucketing (logK > 0): band i covers -// [floor*aⁱ, floor*a^(i+1)) with a = e^logK and floor = scale, so an RTT exactly on -// a band boundary (floor*e^(logK*i)) lands in band i (glyph rttBars[i]) and one past +// TestRttGlyphLog verifies logarithmic bucketing (lnBase > 0): band i covers +// [floor*aⁱ, floor*a^(i+1)) with a = e^lnBase and floor = scale, so an RTT exactly on +// a band boundary (floor*e^(lnBase*i)) lands in band i (glyph rttBars[i]) and one past // the last band overflows to "█". Boundary inputs are generated with math.Exp so the // floating-point edges (which boundaryEpsilon repairs) are exercised. func TestRttGlyphLog(t *testing.T) { exp := math.Exp cases := []struct { - name string - rtt float64 - scale float64 - logK int - want string + name string + rtt float64 + scale float64 + lnBase float64 + want string }{ - // logK=1 (base e), floor 1.0 — exact band boundaries. + // lnBase=1 (base e), floor 1.0 — exact band boundaries. {"k1_below_floor", 0.5, 1.0, 1, "▁"}, {"k1_at_floor", 1.0, 1.0, 1, "▁"}, {"k1_e1", exp(1), 1.0, 1, "▂"}, @@ -240,7 +240,7 @@ func TestRttGlyphLog(t *testing.T) { // Mid-band (not on a boundary) lands in the lower band: floor(2.5) = 2. {"k1_mid_2_3", exp(2.5), 1.0, 1, "▃"}, - // logK=2 (base e²), floor 1.0. + // lnBase=2 (base e²), floor 1.0. {"k2_at_floor", 1.0, 1.0, 2, "▁"}, {"k2_e2", exp(2), 1.0, 2, "▂"}, {"k2_e4", exp(4), 1.0, 2, "▃"}, @@ -259,23 +259,23 @@ func TestRttGlyphLog(t *testing.T) { {"zero_rtt", 0.0, 1.0, 1, "▁"}, {"neg_floor", exp(3), -1.0, 1, "▁"}, - // logK=0 stays linear even through this table. + // lnBase=0 stays linear even through this table. {"k0_linear_9_scale10", 9.0, 10.0, 0, "▁"}, {"k0_linear_70_scale10", 70.0, 10.0, 0, "█"}, } for _, c := range cases { t.Run(c.name, func(t *testing.T) { res := ping.Result{Success: true, Code: ping.Success, RTT: c.rtt} - if got := Glyph(res, c.scale, c.logK); got != c.want { - t.Errorf("Glyph(RTT %v, scale %v, logK %d) = %q, want %q", - c.rtt, c.scale, c.logK, got, c.want) + if got := Glyph(res, c.scale, c.lnBase); got != c.want { + t.Errorf("Glyph(RTT %v, scale %v, lnBase %g) = %q, want %q", + c.rtt, c.scale, c.lnBase, got, c.want) } }) } } // TestRttGlyphLogBoundaryStability checks the log boundary contract across non-unit -// floors and both factors: an RTT exactly on band boundary i (floor*e^(logK*i)) lands +// floors and both factors: an RTT exactly on band boundary i (floor*e^(lnBase*i)) lands // in band i (rttBars[i]) for i in 0..len(rttBars)-1 and overflows to "█" at // i == len(rttBars). These particular floors land on exact integer steps (so the // boundaryEpsilon regression itself is guarded by TestRttGlyphLinearBoundary, where @@ -283,9 +283,9 @@ func TestRttGlyphLog(t *testing.T) { func TestRttGlyphLogBoundaryStability(t *testing.T) { floors := []float64{0.3, 1.0, 2.5, 7.0, 0.001} for _, floor := range floors { - for _, logK := range []int{1, 2} { + for _, lnBase := range []float64{1, 2} { for i := 0; i <= len(rttBars); i++ { - rtt := floor * math.Exp(float64(logK*i)) + rtt := floor * math.Exp(lnBase*float64(i)) want := "█" if i < len(rttBars) { @@ -293,9 +293,9 @@ func TestRttGlyphLogBoundaryStability(t *testing.T) { } res := ping.Result{Success: true, Code: ping.Success, RTT: rtt} - if got := Glyph(res, floor, logK); got != want { - t.Errorf("floor=%v logK=%d band=%d (rtt=%v): Glyph = %q, want %q", - floor, logK, i, rtt, got, want) + if got := Glyph(res, floor, lnBase); got != want { + t.Errorf("floor=%v lnBase=%g band=%d (rtt=%v): Glyph = %q, want %q", + floor, lnBase, i, rtt, got, want) } } } @@ -316,7 +316,7 @@ func TestGlyphFailureCodesLogMode(t *testing.T) { } for _, c := range cases { if got := Glyph(c.res, 1.0, 1); got != c.want { - t.Errorf("Glyph(%+v, logK=1) = %q, want %q", c.res, got, c.want) + t.Errorf("Glyph(%+v, lnBase=1) = %q, want %q", c.res, got, c.want) } } } diff --git a/internal/tui/model.go b/internal/tui/model.go index d321613..0904e7b 100644 --- a/internal/tui/model.go +++ b/internal/tui/model.go @@ -59,7 +59,7 @@ type Model struct { visible map[string]bool // per-column visibility (config defaults + 'm' toggle). scale float64 // RTT-bar ms-per-step (the window floor in log mode); adjusted live with up/down. - logK int // RTT-scale log factor: 0 = linear, 1 = base e, 2 = base e²; cycled with 'l'. + logIdx int // index into logFactors (0 = linear); cycled with 'l'. The selected LnBase drives Glyph. precIdx int // index into precisionModes for the stat columns; cycled with 'p'. scrollTop int // first visible row when the list exceeds the viewport; moved with j/k/g/G/PgUp/PgDn. @@ -79,10 +79,6 @@ type Model struct { warnings []string // startup warnings (e.g. rp_filter, IPv6 nexthop ignored). } -// fallbackScale is the RTT-bar scale New uses when Options.Scale is unset or degenerate -// (a caller bypassing resolveScale); it matches the CLI default so behavior is uniform. -const fallbackScale = 10 - // New builds the initial model from parsed specs and options. The error return is // retained for call-site stability; New currently never fails (a target whose config // cannot be built degrades to a permanent-failure row rather than aborting). @@ -91,10 +87,11 @@ func New(specs []config.TargetSpec, opts Options) (Model, error) { // Normalize the scale like Cols: a caller bypassing resolveScale (a test or an // embedding) may pass a zero/degenerate Options.Scale, which would otherwise make - // rttGlyph's scale<=0 guard flatten every bar to ▁. + // rttGlyph's scale<=0 guard flatten every bar to ▁. config.DefaultScale is the same + // fallback the CLI resolver uses, so behavior is uniform. scale := opts.Scale if !config.ValidScale(scale) { - scale = fallbackScale + scale = config.DefaultScale } return Model{ @@ -341,7 +338,7 @@ func (m Model) adjustRTTScale(key string) Model { case "down": m.scale = scaleDown(m.scale) case "l": - m.logK = (m.logK + 1) % len(logFactors) + m.logIdx = (m.logIdx + 1) % len(logFactors) default: // Unreachable: handleViewKey routes only up/down/l here. } @@ -385,17 +382,26 @@ func (m Model) adjustCols(key string) Model { return m } -// logFactors is the single source of the 'l'-key cycle order and the legend labels for -// the RTT-scale log factor: linear (no log), then base e and base e². The slice index -// IS the logK value handed to monitor.Glyph (the ln divisor, so bars bucket on -// e^index), so a new entry must preserve that index==exponent contract — a non-e^n -// factor (e.g. base 10) needs a matching rttGlyph change, not just a label. Labels stay -// ASCII (narrow): "×"/"²" are East-Asian-ambiguous and render 2 cells on CJK terminals, -// desyncing the keys-line width from the renderer's measure. -var logFactors = []struct{ Label string }{ - {"lin"}, - {"xe"}, - {"xe2"}, +// logFactor is one entry in the 'l'-key cycle: a legend label paired with LnBase, the +// log base carried as a value rather than implied by the slice index. Mirrors +// precisionMode (label + behavior in one row). +type logFactor struct { + Label string + // LnBase is the ln of the per-step bucket base, i.e. the divisor in + // ln(rtt/scale)/LnBase handed to monitor.Glyph: 0 = linear (no log), 1 = base e, + // 2 = base e². Because it is data, not the index, a new factor is just a row — base + // 10 would be {"x10", math.Log(10)} — with no rttGlyph change. Labels stay ASCII + // (narrow): "×"/"²" are East-Asian-ambiguous and render 2 cells on CJK terminals, + // desyncing the keys-line width from the renderer's measure. + LnBase float64 +} + +// logFactors is the single source of the 'l'-key cycle order, its legend labels and the +// log base each selects: linear (no log), then base e and base e². +var logFactors = []logFactor{ + {"lin", 0}, + {"xe", 1}, + {"xe2", 2}, } // scaleSteps is the ladder the up/down keys move the RTT-bar scale through (ms); it diff --git a/internal/tui/view.go b/internal/tui/view.go index 9324b95..27f472d 100644 --- a/internal/tui/view.go +++ b/internal/tui/view.go @@ -2,6 +2,7 @@ package tui import ( "fmt" + "strconv" "strings" "github.com/charmbracelet/lipgloss" @@ -83,12 +84,15 @@ func (m Model) View() string { func (m Model) keysLine() string { // In log mode the scale value is the window floor (not ms-per-step) and the factor // sets the width, so relabel and surface the factor up front (clipped last, like the - // cols count below). + // cols count below). The mode is read from the selected factor's LnBase, not the + // index, so the linear/log split stays tied to the table. + f := activeLogFactor(m.logIdx) + var s string - if m.logK > 0 { - s = rear + fmt.Sprintf("RTT floor %gms %s.", m.scale, logLabel(m.logK)) + if f.LnBase > 0 { + s = rear + fmt.Sprintf("RTT floor %sms %s.", scaleLabel(m.scale), f.Label) } else { - s = rear + fmt.Sprintf("RTT Scale %gms.", m.scale) + s = rear + fmt.Sprintf("RTT Scale %sms.", scaleLabel(m.scale)) } // The effective-vs-requested column count sits near the front, before the long @@ -107,14 +111,22 @@ func (m Model) keysLine() string { return s } -// logLabel renders the RTT-scale log factor for logK as its short legend label -// ("lin", "xe", "xe2"), reading the single logFactors table. -func logLabel(k int) string { - if k < 0 || k >= len(logFactors) { - return logFactors[0].Label +// activeLogFactor returns the RTT-scale log factor at index i from the single +// logFactors table, clamping a stray index to the linear entry so a corrupt logIdx can +// never panic the render. Callers read both its Label (legend) and LnBase (mode/base). +func activeLogFactor(i int) logFactor { + if i < 0 || i >= len(logFactors) { + return logFactors[0] } - return logFactors[k].Label + return logFactors[i] +} + +// scaleLabel formats an RTT-bar scale for the keys line. FormatFloat with 'f' avoids +// %g's exponent switch (an explicit -s 1000000 would otherwise read "1e+06ms") while +// the shortest-precision (-1) keeps ladder values byte-identical: "0.01", "10", "100". +func scaleLabel(scale float64) string { + return strconv.FormatFloat(scale, 'f', -1, 64) } // scrollStatus is the one-line position indicator shown below the row window when @@ -233,7 +245,7 @@ func (m Model) targetLine(idx int, t *monitor.Target) string { var g strings.Builder for _, res := range t.Results(m.resW) { - ch := monitor.Glyph(res, m.scale, m.logK) + ch := monitor.Glyph(res, m.scale, activeLogFactor(m.logIdx).LnBase) if monitor.IsFailGlyph(ch) { g.WriteString(styleDown.Render(ch)) } else { diff --git a/internal/tui/view_test.go b/internal/tui/view_test.go index 885e6f5..a533fd1 100644 --- a/internal/tui/view_test.go +++ b/internal/tui/view_test.go @@ -677,8 +677,8 @@ func TestScaleRebucketsExistingBar(t *testing.T) { } // TestLogModeKey cycles the 'l' key (linear -> base e -> base e² -> linear), asserting -// the footer relabels to floor mode and surfaces the factor up front (×e / ×e²) and -// wraps back. This is the regression guard for the 'l' wiring and logLabel. +// the footer relabels to floor mode and surfaces the factor up front (xe / xe2) and +// wraps back. This is the regression guard for the 'l' wiring and the logFactors lookup. func TestLogModeKey(t *testing.T) { specs := []config.TargetSpec{{Name: "h", Addr: "1.2.3.4", Relay: map[string]string{}}} @@ -712,7 +712,7 @@ func TestLogModeKey(t *testing.T) { } // TestLogModeRebucketsBar confirms 'l' re-buckets the on-screen bar through the View -// path (targetLine passing logK to monitor.Glyph): the same stored RTT 50 renders ▆ on +// path (targetLine passing the selected LnBase to monitor.Glyph): the same stored RTT 50 renders ▆ on // the linear scale 10 but ▂ in log ×e (ln(50/10)≈1.6, band 1). func TestLogModeRebucketsBar(t *testing.T) { specs := []config.TargetSpec{{Name: "h", Addr: "1.2.3.4", Relay: map[string]string{}}} @@ -794,7 +794,7 @@ func TestReloadPreservesScaleAndPrecision(t *testing.T) { // scale/precision/log-factor are preserved (the documented, intentional asymmetry). _, out := drive(t, m, reloadMsg{}) - // The trailing '.' makes this fail if a reload bug advances logK 1->2 (xe -> xe2). + // The trailing '.' makes this fail if a reload bug advances logIdx 1->2 (xe -> xe2). if !strings.Contains(out, "RTT floor 5ms xe.") || !strings.Contains(out, "(p)recision[ms.1]") { t.Errorf("reload should preserve the live scale/precision/log-factor\n---\n%s", out) } From b52380bc7e56cc462b0d6efcd2950f5012a8c773 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=B5=90=E5=8F=8B?= Date: Sat, 13 Jun 2026 10:15:25 +0900 Subject: [PATCH 6/9] =?UTF-8?q?fix:=20=E3=82=B3=E3=83=BC=E3=83=89=E3=83=AC?= =?UTF-8?q?=E3=83=93=E3=83=A5=E3=83=BC=E6=8C=87=E6=91=98=E5=AF=BE=E5=BF=9C?= =?UTF-8?q?=20=E2=80=94=20scale=20=E7=AF=84=E5=9B=B2=E3=82=AC=E3=83=BC?= =?UTF-8?q?=E3=83=89=E3=83=BB=E7=84=A1=E5=8A=B9=20-s=20=E8=AD=A6=E5=91=8A?= =?UTF-8?q?=E3=83=BB=E5=8D=98=E4=B8=80=E6=83=85=E5=A0=B1=E6=BA=90=E5=8C=96?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit /code-review ultra(xhigh) 第2回の確認済み指摘 9 件に対応。 正確性: - ValidScale を実用範囲 [1e-4, 1e6] ms に限定。極端な有限値(例 -s 1e-300)が ① scaleLabel('f',-1) を約300文字に膨張させ keys 凡例を画面外へ追い出す ② 全 RTT を最終バンド超過にして結果バーを全部 █ にする ── を両方封じる。 範囲判定が IsInf を包含するため math 依存も解消。README に実用範囲を明記。 - 明示した無効な -s(inf/nan/負数/範囲外)を起動時警告として可視化。フォールバック 挙動(ValidScale で却下 → config/既定)は不変のまま、これまで黙って捨てていた入力を scaleWarning(cli, resolved) 経由で Options.Warnings に載せ、TUI 先頭に表示する。 旧 int フラグでは inf/nan がハード parse エラーだった回帰の診断を取り戻す。 クリーンアップ(単一情報源化): - config.ScaleOrDefault に「無効→既定」正規化を集約し、resolveScale 末尾と tui.New の重複を解消。 - activeLogFactor(i) 自由関数を Model メソッド m.logFactor() に(precMode に倣う)。 呼び出し側が誤インデックスを渡す余地を消す。 - targetLine のグリフループから m.logFactor() をループ外へホイスト (R×W 回/フレームの冗長なクランプ+構造体コピーを除去)。 - -s/--scale の usage を config.DefaultScale から生成(リテラル "10" の乖離を防止)。 - logFactors の doc を明確化(index 0 の "lin" は巡回スロットの placeholder で 非描画、線形モードは "RTT Scale" の文言で自己ラベルする旨)。 テスト: - TestValidScale / TestScaleOrDefault / TestScaleWarning と Options.Warnings の 描画テストを追加(範囲境界・極端値・inf/nan・警告の可視化)。 - TestScaleStepKeys の Down 押下回数と床ラベルを len(scaleSteps)/scaleLabel から導出。 - TestRttGlyphLog を TestRttGlyphLogBoundaryStability のスイープ非重複ケースに削減 (床未満・バンド中間・巨大オーバーフロー・ガード・線形パススルー)。 make check 全通過(tidy-check/fmt-check/vet/lint/test)・test-race 通過。go.sum 不変。 --- README.md | 2 +- cmd/deadman/main.go | 40 ++++++++++++++++++++------ cmd/deadman/main_test.go | 30 ++++++++++++++++++++ internal/config/config.go | 33 ++++++++++++++++++---- internal/config/config_test.go | 49 ++++++++++++++++++++++++++++++++ internal/monitor/target_test.go | 48 +++++++++---------------------- internal/tui/model.go | 25 +++++++++++------ internal/tui/view.go | 19 +++++++------ internal/tui/view_test.go | 50 ++++++++++++++++++++++----------- 9 files changed, 213 insertions(+), 83 deletions(-) diff --git a/README.md b/README.md index ee94fed..878b9cd 100644 --- a/README.md +++ b/README.md @@ -235,7 +235,7 @@ precision ms.1 統計値の表示精度(ms / ms.1 / ms.2 / ms.3 のいず `.N` の桁数を増やすほどサブミリ秒の差が見えますが列幅も広がります。 実行中は `p` キーで循環します。 -`scale` はコマンドラインの `-s` / `--scale` と同じ設定で、**明示した `-s` が `scale` ディレクティブより優先**されます(どちらも無ければ既定 10。小数値も指定でき、サブミリ秒のスケールに対応します)。 +`scale` はコマンドラインの `-s` / `--scale` と同じ設定で、**明示した `-s` が `scale` ディレクティブより優先**されます(どちらも無ければ既定 10。小数値も指定でき、サブミリ秒のスケールに対応します)。実用範囲(おおよそ 0.0001〜1000000 ms)を外れた極端な値・不正値(0・負数・`inf`・`nan`)は無視され、既定 10 に戻ります。 `precision` にコマンドラインフラグはありません。 再読み込みをまたいだ値の保持については「[操作](#操作)」を参照してください。 diff --git a/cmd/deadman/main.go b/cmd/deadman/main.go index 160fc19..8cc5014 100644 --- a/cmd/deadman/main.go +++ b/cmd/deadman/main.go @@ -28,18 +28,32 @@ var version = "dev" // config scale can take effect. func resolveScale(cli, cfg float64) float64 { // Both inputs go through the same config.ValidScale predicate the "scale" directive - // uses, so a non-finite value (e.g. -s Inf, which flag.Float64 accepts, or a cfg - // built outside the directive parser) is rejected rather than flattening every bar; - // the fallback to the next source then applies. + // uses, so a non-finite or out-of-range value (e.g. -s Inf, which flag.Float64 + // accepts, or a cfg built outside the directive parser) is rejected rather than + // flattening every bar. An explicit CLI -s wins; otherwise config.ScaleOrDefault + // applies the shared "invalid → default" fallback to the config value. if config.ValidScale(cli) { return cli } - if config.ValidScale(cfg) { - return cfg + return config.ScaleOrDefault(cfg) +} + +// scaleWarning returns an operator-facing warning when an explicit -s value was rejected +// as unusable, or "" when the CLI scale was unset (0) or usable. resolved is the scale +// resolveScale chose, so the message names what is actually in effect — a valid config +// "scale" or the default — rather than assuming the default. The probe still runs (the +// invalid value is dropped, not fatal); this only surfaces the silently-ignored input. +func scaleWarning(cli, resolved float64) string { + if cli == 0 || config.ValidScale(cli) { + return "" } - return config.DefaultScale + return fmt.Sprintf( + "-s %g ignored: not a usable RTT-bar scale (finite, ~0.0001..1000000 ms); using %gms instead", + cli, + resolved, + ) } // parseArgs parses the command line into TUI options. Flags may appear before or @@ -48,8 +62,12 @@ func resolveScale(cli, cfg float64) float64 { // configfile intermix. func parseArgs(args []string) (tui.Options, error) { fs := flag.NewFlagSet("deadman", flag.ContinueOnError) - scale := fs.Float64("s", 0, "scale of ping RTT bar gap (ms, default 10, decimals allowed)") - fs.Float64Var(scale, "scale", 0, "scale of ping RTT bar gap (ms, default 10, decimals allowed)") + scaleUsage := fmt.Sprintf( + "scale of ping RTT bar gap (ms, default %g, decimals allowed)", + config.DefaultScale, + ) + scale := fs.Float64("s", 0, scaleUsage) + fs.Float64Var(scale, "scale", 0, scaleUsage) async := fs.Bool("a", false, "send ping asynchronously") fs.BoolVar(async, "async-mode", false, "send ping asynchronously") blink := fs.Bool("b", false, "blink arrow in async mode") @@ -141,7 +159,13 @@ func main() { } opts.Columns = cfg.Columns + rawScale := opts.Scale opts.Scale = resolveScale(opts.Scale, cfg.Scale) + + if w := scaleWarning(rawScale, opts.Scale); w != "" { + opts.Warnings = append(opts.Warnings, w) + } + opts.Precision = cfg.Precision opts.Cols = resolveCols(opts.Cols, cfg.Cols) opts.Version = version diff --git a/cmd/deadman/main_test.go b/cmd/deadman/main_test.go index 46d5c67..17f9322 100644 --- a/cmd/deadman/main_test.go +++ b/cmd/deadman/main_test.go @@ -191,3 +191,33 @@ func TestResolveCols(t *testing.T) { }) } } + +// TestScaleWarning checks that an explicitly-passed, unusable -s value yields a warning +// (the value is still dropped and the probe runs), while an unset or usable -s is silent. +func TestScaleWarning(t *testing.T) { + cases := []struct { + name string + cli, resolved float64 + wantWarn bool + }{ + {"unset cli is silent", 0, 10, false}, + {"valid cli is silent", 5, 5, false}, + {"inf cli warns", math.Inf(1), 10, true}, + {"nan cli warns", math.NaN(), 10, true}, + {"negative cli warns", -5, 10, true}, + {"out-of-range cli warns", 1e-300, 10, true}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + if got := scaleWarning(c.cli, c.resolved); (got != "") != c.wantWarn { + t.Errorf( + "scaleWarning(%g, %g) = %q, wantWarn=%v", + c.cli, + c.resolved, + got, + c.wantWarn, + ) + } + }) + } +} diff --git a/internal/config/config.go b/internal/config/config.go index dc1b8c6..1f49f36 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -10,7 +10,6 @@ package config import ( "bufio" "io" - "math" "regexp" "strconv" "strings" @@ -96,12 +95,23 @@ var directives = map[string]func(cfg *Config, args []string){ }, } -// ValidScale reports whether v is a usable RTT-bar scale: strictly positive and finite. -// It is the single predicate shared by the "scale" directive, the CLI -s resolver -// (cmd/deadman) and the TUI's startup normalization, so inf/nan/non-positive rejection -// cannot drift between them. +// Scale bounds. A scale outside [minScale, maxScale] is unusable, not merely degenerate: +// below minScale every real RTT overflows the top band (the bar is uniformly "█") and the +// shortest-decimal footer label balloons toward hundreds of characters (FormatFloat 'f' +// of 1e-300 is ~300 digits), shoving the key legend off-screen; above maxScale every bar +// collapses to the floor. minScale (0.1 µs) is far finer and maxScale (1000 s) far coarser +// than any real network bar, so the usable window stays generous while nonsense is rejected. +const ( + minScale = 1e-4 + maxScale = 1e6 +) + +// ValidScale reports whether v is a usable RTT-bar scale: within [minScale, maxScale], +// which also excludes NaN, ±Inf, zero and negatives. It is the single predicate shared by +// the "scale" directive, the CLI -s resolver (cmd/deadman) and the TUI's startup +// normalization, so the accept/reject boundary cannot drift between them. func ValidScale(v float64) bool { - return v > 0 && !math.IsInf(v, 0) + return v >= minScale && v <= maxScale } // DefaultScale is the RTT-bar scale used when neither the CLI -s flag nor a "scale" @@ -110,6 +120,17 @@ func ValidScale(v float64) bool { // hardcoding it — the same single-source rationale as ValidScale. const DefaultScale = 10.0 +// ScaleOrDefault returns v when it is a usable scale and DefaultScale otherwise, so the +// "invalid → default" normalization lives in one place rather than being repeated by the +// CLI resolver (resolveScale) and the TUI's New. +func ScaleOrDefault(v float64) float64 { + if ValidScale(v) { + return v + } + + return DefaultScale +} + var reSeparator = regexp.MustCompile(`^-+$`) // relayKeys is the set of attribute keys routed into a target's relay map. "netns" is diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 3ba29ea..7fa780e 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -1,10 +1,59 @@ package config import ( + "math" "strings" "testing" ) +// TestValidScale pins the usable-scale window: the [minScale, maxScale] bounds (and the +// implicit rejection of zero, negatives, NaN and ±Inf). Extreme finite values are +// rejected too — below minScale the footer label balloons and every bar overflows. +func TestValidScale(t *testing.T) { + cases := []struct { + v float64 + ok bool + }{ + {10, true}, + {0.01, true}, // sub-ms ladder bottom. + {minScale, true}, // inclusive lower bound. + {maxScale, true}, // inclusive upper bound. + {0, false}, + {-5, false}, + {1e-300, false}, // far below minScale: ~300-char footer label, bar all █. + {1e300, false}, // far above maxScale. + {math.Inf(1), false}, + {math.Inf(-1), false}, + {math.NaN(), false}, + } + for _, c := range cases { + if got := ValidScale(c.v); got != c.ok { + t.Errorf("ValidScale(%g) = %v, want %v", c.v, got, c.ok) + } + } +} + +// TestScaleOrDefault confirms the shared "invalid → default" normalization: a usable +// value passes through, anything ValidScale rejects becomes DefaultScale. +func TestScaleOrDefault(t *testing.T) { + cases := []struct { + v, want float64 + }{ + {5, 5}, + {0.5, 0.5}, + {0, DefaultScale}, + {-1, DefaultScale}, + {1e-300, DefaultScale}, + {math.Inf(1), DefaultScale}, + {math.NaN(), DefaultScale}, + } + for _, c := range cases { + if got := ScaleOrDefault(c.v); got != c.want { + t.Errorf("ScaleOrDefault(%g) = %g, want %g", c.v, got, c.want) + } + } +} + const sample = `# # deadman config # diff --git a/internal/monitor/target_test.go b/internal/monitor/target_test.go index 8542ae6..cf85df3 100644 --- a/internal/monitor/target_test.go +++ b/internal/monitor/target_test.go @@ -211,11 +211,11 @@ func TestRttGlyphLinearBoundary(t *testing.T) { } } -// TestRttGlyphLog verifies logarithmic bucketing (lnBase > 0): band i covers -// [floor*aⁱ, floor*a^(i+1)) with a = e^lnBase and floor = scale, so an RTT exactly on -// a band boundary (floor*e^(lnBase*i)) lands in band i (glyph rttBars[i]) and one past -// the last band overflows to "█". Boundary inputs are generated with math.Exp so the -// floating-point edges (which boundaryEpsilon repairs) are exercised. +// TestRttGlyphLog covers the log-mode cases that TestRttGlyphLogBoundaryStability's +// exhaustive boundary sweep does not: an RTT below the floor, a mid-band value (not on a +// boundary), a far overflow, the zero/negative guards, and the lnBase==0 linear +// passthrough. The exact band boundaries (floor*e^(lnBase*i) → rttBars[i]) are pinned by +// the sweep across several floors and both factors, so they are not duplicated here. func TestRttGlyphLog(t *testing.T) { exp := math.Exp @@ -226,42 +226,20 @@ func TestRttGlyphLog(t *testing.T) { lnBase float64 want string }{ - // lnBase=1 (base e), floor 1.0 — exact band boundaries. - {"k1_below_floor", 0.5, 1.0, 1, "▁"}, - {"k1_at_floor", 1.0, 1.0, 1, "▁"}, - {"k1_e1", exp(1), 1.0, 1, "▂"}, - {"k1_e2", exp(2), 1.0, 1, "▃"}, - {"k1_e3", exp(3), 1.0, 1, "▄"}, - {"k1_e4", exp(4), 1.0, 1, "▅"}, - {"k1_e5", exp(5), 1.0, 1, "▆"}, - {"k1_e6", exp(6), 1.0, 1, "▇"}, - {"k1_e7", exp(7), 1.0, 1, "█"}, - {"k1_huge", 1e9, 1.0, 1, "█"}, - // Mid-band (not on a boundary) lands in the lower band: floor(2.5) = 2. - {"k1_mid_2_3", exp(2.5), 1.0, 1, "▃"}, - - // lnBase=2 (base e²), floor 1.0. - {"k2_at_floor", 1.0, 1.0, 2, "▁"}, - {"k2_e2", exp(2), 1.0, 2, "▂"}, - {"k2_e4", exp(4), 1.0, 2, "▃"}, - {"k2_e6", exp(6), 1.0, 2, "▄"}, - {"k2_e8", exp(8), 1.0, 2, "▅"}, - {"k2_e10", exp(10), 1.0, 2, "▆"}, - {"k2_e12", exp(12), 1.0, 2, "▇"}, - {"k2_e14", exp(14), 1.0, 2, "█"}, - - // Non-unit floors on exact band boundaries (these land on integer steps). - {"floor03_e2", 0.3 * exp(2), 0.3, 1, "▃"}, - {"floor25_e4", 2.5 * exp(4), 2.5, 1, "▅"}, - {"subms_floor_e3", 0.001 * exp(3), 0.001, 1, "▄"}, + // Below the floor (step < 0 in log space) clamps to the lowest bar. + {"below_floor", 0.5, 1.0, 1, "▁"}, + // Mid-band (not on a boundary) lands in the lower band: floor(2.5) = band 2. + {"mid_band_2_3", exp(2.5), 1.0, 1, "▃"}, + // Far past the last boundary still overflows to the full block. + {"far_overflow", 1e9, 1.0, 1, "█"}, // Guards: zero/negative RTT and a degenerate floor fall back to ▁. {"zero_rtt", 0.0, 1.0, 1, "▁"}, {"neg_floor", exp(3), -1.0, 1, "▁"}, // lnBase=0 stays linear even through this table. - {"k0_linear_9_scale10", 9.0, 10.0, 0, "▁"}, - {"k0_linear_70_scale10", 70.0, 10.0, 0, "█"}, + {"linear_9_scale10", 9.0, 10.0, 0, "▁"}, + {"linear_70_scale10", 70.0, 10.0, 0, "█"}, } for _, c := range cases { t.Run(c.name, func(t *testing.T) { diff --git a/internal/tui/model.go b/internal/tui/model.go index 0904e7b..72cb63f 100644 --- a/internal/tui/model.go +++ b/internal/tui/model.go @@ -38,6 +38,7 @@ type Options struct { Version string // build version label shown in the title bar (Makefile -ldflags); "" = "dev". Columns map[string]bool // per-column visibility overrides (config file). Cols int // requested newspaper-column count (CLI -c/--split, config "split"); <=1 = single. + Warnings []string // CLI-level startup warnings (e.g. an invalid -s value), shown above the rows. } // Model is the Bubble Tea model. @@ -87,12 +88,14 @@ func New(specs []config.TargetSpec, opts Options) (Model, error) { // Normalize the scale like Cols: a caller bypassing resolveScale (a test or an // embedding) may pass a zero/degenerate Options.Scale, which would otherwise make - // rttGlyph's scale<=0 guard flatten every bar to ▁. config.DefaultScale is the same - // fallback the CLI resolver uses, so behavior is uniform. - scale := opts.Scale - if !config.ValidScale(scale) { - scale = config.DefaultScale - } + // rttGlyph's scale<=0 guard flatten every bar to ▁. config.ScaleOrDefault is the same + // "invalid → default" helper the CLI resolver's fallback uses, so behavior is uniform. + scale := config.ScaleOrDefault(opts.Scale) + + // CLI-level warnings (e.g. an invalid -s) lead, then the per-target startup and build + // warnings. Clone so appending never mutates the caller's slice. + warnings := append(slices.Clone(opts.Warnings), startupWarnings(specs)...) + warnings = append(warnings, buildWarns...) return Model{ rows: rows, @@ -101,7 +104,7 @@ func New(specs []config.TargetSpec, opts Options) (Model, error) { precIdx: precisionIndex(opts.Precision), hostInfo: hostInfo(), visible: buildVisible(opts.Columns), - warnings: append(startupWarnings(specs), buildWarns...), + warnings: warnings, cols: max(opts.Cols, 1), }, nil } @@ -396,8 +399,12 @@ type logFactor struct { LnBase float64 } -// logFactors is the single source of the 'l'-key cycle order, its legend labels and the -// log base each selects: linear (no log), then base e and base e². +// logFactors is the single source of the 'l'-key cycle order, the log base each entry +// selects, and the floor-mode legend labels for the log factors: linear (no log), then +// base e and base e². Index 0's "lin" label is a cycle-slot placeholder, not rendered: +// keysLine labels linear mode with its own "RTT Scale" wording (vs "RTT floor … xe"), +// since linear shows ms-per-step while a log factor shows the window floor. So the table +// is the single source of the log-factor labels (index > 0); the linear row is self-named. var logFactors = []logFactor{ {"lin", 0}, {"xe", 1}, diff --git a/internal/tui/view.go b/internal/tui/view.go index 27f472d..d92f206 100644 --- a/internal/tui/view.go +++ b/internal/tui/view.go @@ -86,7 +86,7 @@ func (m Model) keysLine() string { // sets the width, so relabel and surface the factor up front (clipped last, like the // cols count below). The mode is read from the selected factor's LnBase, not the // index, so the linear/log split stays tied to the table. - f := activeLogFactor(m.logIdx) + f := m.logFactor() var s string if f.LnBase > 0 { @@ -111,15 +111,15 @@ func (m Model) keysLine() string { return s } -// activeLogFactor returns the RTT-scale log factor at index i from the single -// logFactors table, clamping a stray index to the linear entry so a corrupt logIdx can -// never panic the render. Callers read both its Label (legend) and LnBase (mode/base). -func activeLogFactor(i int) logFactor { - if i < 0 || i >= len(logFactors) { +// logFactor returns the active RTT-scale log factor for the model, clamping an out-of- +// range logIdx to the linear entry so a Model not built through New can never panic the +// render (mirrors precMode). Callers read both its Label (legend) and LnBase (mode/base). +func (m Model) logFactor() logFactor { + if m.logIdx < 0 || m.logIdx >= len(logFactors) { return logFactors[0] } - return logFactors[i] + return logFactors[m.logIdx] } // scaleLabel formats an RTT-bar scale for the keys line. FormatFloat with 'f' avoids @@ -244,8 +244,11 @@ func (m Model) targetLine(idx int, t *monitor.Target) string { var g strings.Builder + // logIdx is loop-invariant, so resolve the log base once rather than per glyph + // (this loop runs resultWidth times per row, every frame). + lnBase := m.logFactor().LnBase for _, res := range t.Results(m.resW) { - ch := monitor.Glyph(res, m.scale, activeLogFactor(m.logIdx).LnBase) + ch := monitor.Glyph(res, m.scale, lnBase) if monitor.IsFailGlyph(ch) { g.WriteString(styleDown.Render(ch)) } else { diff --git a/internal/tui/view_test.go b/internal/tui/view_test.go index a533fd1..6b7d4f8 100644 --- a/internal/tui/view_test.go +++ b/internal/tui/view_test.go @@ -593,23 +593,25 @@ func TestScaleStepKeys(t *testing.T) { t.Errorf("after up,up: want scale 20\n---\n%s", out) } - // down past the bottom clamps at the sub-ms floor (0.01ms). From 20ms the ladder - // descends 20→10→5→2→1→0.5→0.2→0.1→0.05→0.02→0.01; press well past the bottom. - m, out = drive(t, m, - tea.KeyMsg{Type: tea.KeyDown}, tea.KeyMsg{Type: tea.KeyDown}, - tea.KeyMsg{Type: tea.KeyDown}, tea.KeyMsg{Type: tea.KeyDown}, - tea.KeyMsg{Type: tea.KeyDown}, tea.KeyMsg{Type: tea.KeyDown}, - tea.KeyMsg{Type: tea.KeyDown}, tea.KeyMsg{Type: tea.KeyDown}, - tea.KeyMsg{Type: tea.KeyDown}, tea.KeyMsg{Type: tea.KeyDown}, - tea.KeyMsg{Type: tea.KeyDown}, tea.KeyMsg{Type: tea.KeyDown}) - if !strings.Contains(out, "RTT Scale 0.01ms") { - t.Errorf("down past the bottom should clamp at 0.01ms\n---\n%s", out) - } - - // further down past the floor stays clamped at 0.01ms. + // down past the bottom clamps at the sub-ms floor (scaleSteps[0]). Pressing Down + // len(scaleSteps) times reaches the floor from any rung, so the count and the + // expected floor label both track the ladder instead of hardcoded literals. + floor := scaleLabel(scaleSteps[0]) + + downs := make([]tea.Msg, len(scaleSteps)) + for i := range downs { + downs[i] = tea.KeyMsg{Type: tea.KeyDown} + } + + m, out = drive(t, m, downs...) + if !strings.Contains(out, "RTT Scale "+floor+"ms") { + t.Errorf("down past the bottom should clamp at %sms\n---\n%s", floor, out) + } + + // further down past the floor stays clamped at the floor. _, out = drive(t, m, tea.KeyMsg{Type: tea.KeyDown}) - if !strings.Contains(out, "RTT Scale 0.01ms") { - t.Errorf("further down past the floor must stay at 0.01ms\n---\n%s", out) + if !strings.Contains(out, "RTT Scale "+floor+"ms") { + t.Errorf("further down past the floor must stay at %sms\n---\n%s", floor, out) } } @@ -1203,3 +1205,19 @@ func TestViewFitsTerminalHeight(t *testing.T) { _, sample := drive(t, m, tea.WindowSizeMsg{Width: 120, Height: 20}) t.Logf("sample render (120x20, 50 targets):\n%s", sample) } + +// TestOptionsWarningsSurface confirms a CLI-level warning passed via Options.Warnings is +// rendered (with the "! " prefix) ahead of the rows, like the per-target startup warnings. +func TestOptionsWarningsSurface(t *testing.T) { + specs := []config.TargetSpec{{Name: "h", Addr: "1.2.3.4", Relay: map[string]string{}}} + + m, err := New(specs, Options{Scale: 10, Warnings: []string{"cli warn xyz"}}) + if err != nil { + t.Fatal(err) + } + + _, out := drive(t, m, tea.WindowSizeMsg{Width: 120, Height: 40}) + if !strings.Contains(out, "! cli warn xyz") { + t.Errorf("Options.Warnings should render with a '! ' prefix\n---\n%s", out) + } +} From 0180e5c901849008618c0353990435be5bb6fe88 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=B5=90=E5=8F=8B?= Date: Sat, 13 Jun 2026 11:48:55 +0900 Subject: [PATCH 7/9] =?UTF-8?q?fix:=20=E3=82=B3=E3=83=BC=E3=83=89=E3=83=AC?= =?UTF-8?q?=E3=83=93=E3=83=A5=E3=83=BC=E6=8C=87=E6=91=98=E5=AF=BE=E5=BF=9C?= =?UTF-8?q?=20=E2=80=94=20=E8=AD=A6=E5=91=8A=E3=81=AE=E5=8D=98=E4=B8=80?= =?UTF-8?q?=E6=83=85=E5=A0=B1=E6=BA=90=E5=8C=96=E3=83=BB=E3=83=AA=E3=83=AD?= =?UTF-8?q?=E3=83=BC=E3=83=89=E6=AE=8B=E5=AD=98=E3=83=BB=E5=A5=91=E7=B4=84?= =?UTF-8?q?=E6=95=B4=E5=90=88?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit /code-review ultra(xhigh) 第5回の確認済み指摘 5 件に対応(正確性バグなし・全件 low)。 正確性/挙動: - CLI 警告(無効な -s の診断)がリロードで消える不具合を修正。warnings 合成を composeWarnings(cli, target) に一本化し New と handleReload の両方が経由する。 フォールバック中の scale は生きているのに警告だけ消えヘッダーが 1 行ずれる回帰を封じる。 TestOptionsWarningsSurface にリロード後も CLI 警告が残ることの検証を追加。 クリーンアップ(単一情報源化): - resolveScale と scaleWarning の二重 ValidScale 判定を解消。 resolveScale(cli, cfg) (float64, string) がスケールと警告を一度に計算し、 値と警告のペアが規約頼みで乖離する余地を消す。main の rawScale 一時変数も除去。 - 警告文中の有効範囲ハードコード("~0.0001..1000000 ms")を撤廃。 minScale/maxScale を MinScale/MaxScale としてエクスポートし、 strconv.FormatFloat で定数から整形(DefaultScale と同じ単一情報源の方針)。 ドキュメント/コメント整合: - README の scale フォールバック記述を実装に合わせて訂正。不正な -s はまず有効な scale ディレクティブへ、それも無ければ既定 10。無視された -s は起動時警告に出る旨も明記。 - TestParseConfigScaleLenient の古いコメント(存在しない math.IsInf ガード/n>0 チェック)を 単一 ValidScale 範囲判定での却下に書き換え。 テスト: - TestResolveScale を警告込みに拡張し旧 TestScaleWarning を統合。警告が実効値 (using %gms)を名指しすることまで固定化。 make check 全通過(tidy-check/fmt-check/vet/lint/test)・test-race 通過。go.sum 不変。 --- README.md | 2 +- cmd/deadman/main.go | 62 ++++++++++++++--------------- cmd/deadman/main_test.go | 73 +++++++++++++++------------------- internal/config/config.go | 18 +++++---- internal/config/config_test.go | 16 ++++---- internal/tui/model.go | 16 +++++--- internal/tui/view_test.go | 21 ++++++++-- 7 files changed, 111 insertions(+), 97 deletions(-) diff --git a/README.md b/README.md index 878b9cd..1bdf48d 100644 --- a/README.md +++ b/README.md @@ -235,7 +235,7 @@ precision ms.1 統計値の表示精度(ms / ms.1 / ms.2 / ms.3 のいず `.N` の桁数を増やすほどサブミリ秒の差が見えますが列幅も広がります。 実行中は `p` キーで循環します。 -`scale` はコマンドラインの `-s` / `--scale` と同じ設定で、**明示した `-s` が `scale` ディレクティブより優先**されます(どちらも無ければ既定 10。小数値も指定でき、サブミリ秒のスケールに対応します)。実用範囲(おおよそ 0.0001〜1000000 ms)を外れた極端な値・不正値(0・負数・`inf`・`nan`)は無視され、既定 10 に戻ります。 +`scale` はコマンドラインの `-s` / `--scale` と同じ設定で、**明示した `-s` が `scale` ディレクティブより優先**されます(どちらも無ければ既定 10。小数値も指定でき、サブミリ秒のスケールに対応します)。実用範囲(0.0001〜1000000 ms)を外れた極端な値・不正値(0・負数・`inf`・`nan`)は無視され、もう一方の有効な値(不正な `-s` に対しては `scale` ディレクティブ)、それも無ければ既定 10 が使われます。無視された `-s` は起動時に警告として表示されます。 `precision` にコマンドラインフラグはありません。 再読み込みをまたいだ値の保持については「[操作](#操作)」を参照してください。 diff --git a/cmd/deadman/main.go b/cmd/deadman/main.go index 8cc5014..d660439 100644 --- a/cmd/deadman/main.go +++ b/cmd/deadman/main.go @@ -8,6 +8,7 @@ import ( "flag" "fmt" "os" + "strconv" tea "github.com/charmbracelet/bubbletea" @@ -21,38 +22,36 @@ import ( // describe); a plain `go install`/`go build` leaves it at "dev". var version = "dev" -// resolveScale picks the effective RTT-bar scale: an explicit CLI -s wins, else a -// config "scale" directive, else config.DefaultScale. 0 means "unset" for both inputs -// (the -s flag defaults to 0, and a missing or invalid "scale" directive leaves -// cfg.Scale at 0), so passing -s is never indistinguishable from its own default and a -// config scale can take effect. -func resolveScale(cli, cfg float64) float64 { - // Both inputs go through the same config.ValidScale predicate the "scale" directive - // uses, so a non-finite or out-of-range value (e.g. -s Inf, which flag.Float64 - // accepts, or a cfg built outside the directive parser) is rejected rather than - // flattening every bar. An explicit CLI -s wins; otherwise config.ScaleOrDefault - // applies the shared "invalid → default" fallback to the config value. +// resolveScale picks the effective RTT-bar scale and, when an explicit -s value was +// rejected as unusable, an operator-facing warning ("" otherwise): an explicit CLI -s +// wins, else a config "scale" directive, else config.DefaultScale. 0 means "unset" for +// both inputs (the -s flag defaults to 0, and a missing or invalid "scale" directive +// leaves cfg.Scale at 0), so passing -s is never indistinguishable from its own default +// and a config scale can take effect. +// +// Both inputs go through the same config.ValidScale predicate the "scale" directive +// uses, so a non-finite or out-of-range value (e.g. -s Inf, which flag.Float64 accepts) +// is rejected rather than flattening every bar; the rejection is not fatal — the value +// is dropped and the probe runs. Scale and warning are computed in one place so the +// message always names what is actually in effect — a valid config "scale" or the +// default — and cannot drift from the accept/reject decision. +func resolveScale(cli, cfg float64) (float64, string) { if config.ValidScale(cli) { - return cli + return cli, "" } - return config.ScaleOrDefault(cfg) -} + scale := config.ScaleOrDefault(cfg) -// scaleWarning returns an operator-facing warning when an explicit -s value was rejected -// as unusable, or "" when the CLI scale was unset (0) or usable. resolved is the scale -// resolveScale chose, so the message names what is actually in effect — a valid config -// "scale" or the default — rather than assuming the default. The probe still runs (the -// invalid value is dropped, not fatal); this only surfaces the silently-ignored input. -func scaleWarning(cli, resolved float64) string { - if cli == 0 || config.ValidScale(cli) { - return "" + if cli == 0 { // unset -s: nothing to surface. + return scale, "" } - return fmt.Sprintf( - "-s %g ignored: not a usable RTT-bar scale (finite, ~0.0001..1000000 ms); using %gms instead", + return scale, fmt.Sprintf( + "-s %g ignored: not a usable RTT-bar scale (%s..%s ms); using %gms instead", cli, - resolved, + strconv.FormatFloat(config.MinScale, 'f', -1, 64), + strconv.FormatFloat(config.MaxScale, 'f', -1, 64), + scale, ) } @@ -158,18 +157,17 @@ func main() { os.Exit(1) } + scale, warn := resolveScale(opts.Scale, cfg.Scale) opts.Columns = cfg.Columns - rawScale := opts.Scale - opts.Scale = resolveScale(opts.Scale, cfg.Scale) - - if w := scaleWarning(rawScale, opts.Scale); w != "" { - opts.Warnings = append(opts.Warnings, w) - } - + opts.Scale = scale opts.Precision = cfg.Precision opts.Cols = resolveCols(opts.Cols, cfg.Cols) opts.Version = version + if warn != "" { + opts.Warnings = append(opts.Warnings, warn) + } + if opts.LogDir != "" { opts.LogWriter = monitor.NewLogWriter(opts.LogDir) } diff --git a/cmd/deadman/main_test.go b/cmd/deadman/main_test.go index 17f9322..a652b0b 100644 --- a/cmd/deadman/main_test.go +++ b/cmd/deadman/main_test.go @@ -3,8 +3,10 @@ package main import ( "errors" "flag" + "fmt" "math" "os" + "strings" "testing" "github.com/yuu61/deadman/internal/config" @@ -122,27 +124,48 @@ func TestParseArgsRejectsMultipleConfigs(t *testing.T) { } } +// TestResolveScale pins both the effective scale and its companion warning: an explicitly +// passed, unusable -s warns (the value is dropped and the probe runs) while an unset or +// usable -s is silent, and a warning always names the scale actually in effect. func TestResolveScale(t *testing.T) { cases := []struct { name string cli, cfg float64 want float64 + wantWarn bool }{ - {"cli explicit wins over config", 20, 5, 20}, - {"config used when cli unset", 0, 5, 5}, - {"cli used when config unset", 7, 0, 7}, - {"sub-ms cli is honored", 0.5, 0, 0.5}, - {"non-finite cli falls back to config", math.Inf(1), 5, 5}, - {"nan cli falls back to config", math.NaN(), 5, 5}, - {"non-finite cfg falls back to default", 0, math.Inf(1), config.DefaultScale}, - {"nan cfg falls back to default", 0, math.NaN(), config.DefaultScale}, - {"default when both unset", 0, 0, config.DefaultScale}, + {"cli explicit wins over config", 20, 5, 20, false}, + {"config used when cli unset", 0, 5, 5, false}, + {"cli used when config unset", 7, 0, 7, false}, + {"sub-ms cli is honored", 0.5, 0, 0.5, false}, + {"non-finite cli falls back to config and warns", math.Inf(1), 5, 5, true}, + {"nan cli falls back to config and warns", math.NaN(), 5, 5, true}, + {"negative cli falls back to default and warns", -5, 0, config.DefaultScale, true}, + {"out-of-range cli falls back to default and warns", 1e-300, 0, config.DefaultScale, true}, + {"non-finite cfg falls back to default", 0, math.Inf(1), config.DefaultScale, false}, + {"nan cfg falls back to default", 0, math.NaN(), config.DefaultScale, false}, + {"default when both unset", 0, 0, config.DefaultScale, false}, } for _, c := range cases { t.Run(c.name, func(t *testing.T) { - if got := resolveScale(c.cli, c.cfg); got != c.want { + got, warn := resolveScale(c.cli, c.cfg) + if got != c.want { t.Errorf("resolveScale(%g, %g) = %g, want %g", c.cli, c.cfg, got, c.want) } + + if (warn != "") != c.wantWarn { + t.Errorf( + "resolveScale(%g, %g) warning = %q, wantWarn=%v", + c.cli, c.cfg, warn, c.wantWarn, + ) + } + + // The warning must name the scale actually in effect (config or default), + // not assume the default. + if effective := fmt.Sprintf("using %gms", c.want); c.wantWarn && + !strings.Contains(warn, effective) { + t.Errorf("warning %q should contain %q", warn, effective) + } }) } } @@ -191,33 +214,3 @@ func TestResolveCols(t *testing.T) { }) } } - -// TestScaleWarning checks that an explicitly-passed, unusable -s value yields a warning -// (the value is still dropped and the probe runs), while an unset or usable -s is silent. -func TestScaleWarning(t *testing.T) { - cases := []struct { - name string - cli, resolved float64 - wantWarn bool - }{ - {"unset cli is silent", 0, 10, false}, - {"valid cli is silent", 5, 5, false}, - {"inf cli warns", math.Inf(1), 10, true}, - {"nan cli warns", math.NaN(), 10, true}, - {"negative cli warns", -5, 10, true}, - {"out-of-range cli warns", 1e-300, 10, true}, - } - for _, c := range cases { - t.Run(c.name, func(t *testing.T) { - if got := scaleWarning(c.cli, c.resolved); (got != "") != c.wantWarn { - t.Errorf( - "scaleWarning(%g, %g) = %q, wantWarn=%v", - c.cli, - c.resolved, - got, - c.wantWarn, - ) - } - }) - } -} diff --git a/internal/config/config.go b/internal/config/config.go index 1f49f36..c8f76d5 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -95,23 +95,25 @@ var directives = map[string]func(cfg *Config, args []string){ }, } -// Scale bounds. A scale outside [minScale, maxScale] is unusable, not merely degenerate: -// below minScale every real RTT overflows the top band (the bar is uniformly "█") and the +// Scale bounds. A scale outside [MinScale, MaxScale] is unusable, not merely degenerate: +// below MinScale every real RTT overflows the top band (the bar is uniformly "█") and the // shortest-decimal footer label balloons toward hundreds of characters (FormatFloat 'f' -// of 1e-300 is ~300 digits), shoving the key legend off-screen; above maxScale every bar -// collapses to the floor. minScale (0.1 µs) is far finer and maxScale (1000 s) far coarser +// of 1e-300 is ~300 digits), shoving the key legend off-screen; above MaxScale every bar +// collapses to the floor. MinScale (0.1 µs) is far finer and MaxScale (1000 s) far coarser // than any real network bar, so the usable window stays generous while nonsense is rejected. +// Exported so the CLI's rejection warning (cmd/deadman) formats the bounds from these +// constants instead of carrying a prose copy that drifts when the window changes. const ( - minScale = 1e-4 - maxScale = 1e6 + MinScale = 1e-4 + MaxScale = 1e6 ) -// ValidScale reports whether v is a usable RTT-bar scale: within [minScale, maxScale], +// ValidScale reports whether v is a usable RTT-bar scale: within [MinScale, MaxScale], // which also excludes NaN, ±Inf, zero and negatives. It is the single predicate shared by // the "scale" directive, the CLI -s resolver (cmd/deadman) and the TUI's startup // normalization, so the accept/reject boundary cannot drift between them. func ValidScale(v float64) bool { - return v >= minScale && v <= maxScale + return v >= MinScale && v <= MaxScale } // DefaultScale is the RTT-bar scale used when neither the CLI -s flag nor a "scale" diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 7fa780e..8ef6a23 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -6,9 +6,9 @@ import ( "testing" ) -// TestValidScale pins the usable-scale window: the [minScale, maxScale] bounds (and the +// TestValidScale pins the usable-scale window: the [MinScale, MaxScale] bounds (and the // implicit rejection of zero, negatives, NaN and ±Inf). Extreme finite values are -// rejected too — below minScale the footer label balloons and every bar overflows. +// rejected too — below MinScale the footer label balloons and every bar overflows. func TestValidScale(t *testing.T) { cases := []struct { v float64 @@ -16,12 +16,12 @@ func TestValidScale(t *testing.T) { }{ {10, true}, {0.01, true}, // sub-ms ladder bottom. - {minScale, true}, // inclusive lower bound. - {maxScale, true}, // inclusive upper bound. + {MinScale, true}, // inclusive lower bound. + {MaxScale, true}, // inclusive upper bound. {0, false}, {-5, false}, - {1e-300, false}, // far below minScale: ~300-char footer label, bar all █. - {1e300, false}, // far above maxScale. + {1e-300, false}, // far below MinScale: ~300-char footer label, bar all █. + {1e300, false}, // far above MaxScale. {math.Inf(1), false}, {math.Inf(-1), false}, {math.NaN(), false}, @@ -500,8 +500,8 @@ func TestParseConfigSplitLenient(t *testing.T) { func TestParseConfigScaleLenient(t *testing.T) { // A non-numeric, non-positive, or non-finite scale is ignored, leaving Scale unset // (0) so the caller falls back to the CLI/default rather than aborting the parse. - // "scale inf" is the case the math.IsInf guard catches (ParseFloat accepts it and - // +Inf > 0); "scale nan" is caught by the n > 0 check (NaN > 0 is false). + // All of these fall to the single ValidScale range predicate: ParseFloat accepts + // "inf"/"nan", but +Inf fails v <= MaxScale and NaN fails both range comparisons. for _, in := range []string{ "scale abc\n", "scale -3\n", "scale 0\n", "scale\n", "scale inf\n", "scale nan\n", } { diff --git a/internal/tui/model.go b/internal/tui/model.go index 72cb63f..8741080 100644 --- a/internal/tui/model.go +++ b/internal/tui/model.go @@ -92,10 +92,7 @@ func New(specs []config.TargetSpec, opts Options) (Model, error) { // "invalid → default" helper the CLI resolver's fallback uses, so behavior is uniform. scale := config.ScaleOrDefault(opts.Scale) - // CLI-level warnings (e.g. an invalid -s) lead, then the per-target startup and build - // warnings. Clone so appending never mutates the caller's slice. - warnings := append(slices.Clone(opts.Warnings), startupWarnings(specs)...) - warnings = append(warnings, buildWarns...) + warnings := composeWarnings(opts.Warnings, append(startupWarnings(specs), buildWarns...)) return Model{ rows: rows, @@ -109,6 +106,15 @@ func New(specs []config.TargetSpec, opts Options) (Model, error) { }, nil } +// composeWarnings builds the header warning list: CLI-level diagnostics (e.g. an invalid +// -s) lead, then the per-target startup/build warnings. New and handleReload both go +// through it, so a reload that regenerates the target warnings cannot silently drop the +// CLI ones — the rejected flag they describe is still in force-by-fallback after a +// reload. Clones cli so appending never mutates the caller's slice. +func composeWarnings(cli, target []string) []string { + return append(slices.Clone(cli), target...) +} + // buildRows turns specs into rows, returning any per-target construction warnings. // Targets present in existing (matched by Key) are reused so their history/stats // survive a reload. A target whose config cannot be built (e.g. a missing required @@ -453,7 +459,7 @@ func (m Model) handleReload() (tea.Model, tea.Cmd) { if r, ok := loadRows(m.opts.ConfigPath, m.rows); ok { m.rows = r.rows m.visible = buildVisible(r.columns) - m.warnings = r.warnings + m.warnings = composeWarnings(m.opts.Warnings, r.warnings) m = m.recalcWidths() m = m.clampScroll() // a shrunk target set may leave scrollTop past the end. } diff --git a/internal/tui/view_test.go b/internal/tui/view_test.go index 6b7d4f8..b6725fc 100644 --- a/internal/tui/view_test.go +++ b/internal/tui/view_test.go @@ -1207,17 +1207,32 @@ func TestViewFitsTerminalHeight(t *testing.T) { } // TestOptionsWarningsSurface confirms a CLI-level warning passed via Options.Warnings is -// rendered (with the "! " prefix) ahead of the rows, like the per-target startup warnings. +// rendered (with the "! " prefix) ahead of the rows, like the per-target startup warnings, +// and that it survives a reload: the rejected flag it describes is still in force, so a +// reload that regenerates the per-target warnings must re-prepend the CLI ones. func TestOptionsWarningsSurface(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "deadman.conf") + + err := os.WriteFile(path, []byte("h 1.2.3.4\n"), 0o600) + if err != nil { + t.Fatal(err) + } + specs := []config.TargetSpec{{Name: "h", Addr: "1.2.3.4", Relay: map[string]string{}}} - m, err := New(specs, Options{Scale: 10, Warnings: []string{"cli warn xyz"}}) + m, err := New(specs, Options{Scale: 10, ConfigPath: path, Warnings: []string{"cli warn xyz"}}) if err != nil { t.Fatal(err) } - _, out := drive(t, m, tea.WindowSizeMsg{Width: 120, Height: 40}) + m, out := drive(t, m, tea.WindowSizeMsg{Width: 120, Height: 40}) if !strings.Contains(out, "! cli warn xyz") { t.Errorf("Options.Warnings should render with a '! ' prefix\n---\n%s", out) } + + _, out = drive(t, m, reloadMsg{}) + if !strings.Contains(out, "! cli warn xyz") { + t.Errorf("Options.Warnings should survive a reload\n---\n%s", out) + } } From a5f8f16c46ee193f15019e6c7cff794f1fe6e24a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=B5=90=E5=8F=8B?= Date: Sat, 13 Jun 2026 11:59:49 +0900 Subject: [PATCH 8/9] =?UTF-8?q?fix:=20codex=20=E3=83=AC=E3=83=93=E3=83=A5?= =?UTF-8?q?=E3=83=BC=E5=AF=BE=E5=BF=9C=20=E2=80=94=20=E6=98=8E=E7=A4=BA=20?= =?UTF-8?q?-s=200=20=E3=81=AE=E8=AD=A6=E5=91=8A=E6=AC=A0=E8=90=BD=E3=83=BB?= =?UTF-8?q?=E3=83=A9=E3=82=A4=E3=83=96=E5=A4=89=E6=9B=B4=E5=BE=8C=E3=81=AE?= =?UTF-8?q?=E5=8F=A4=E3=81=84=E5=AE=9F=E5=8A=B9=E5=80=A4=E8=A1=A8=E7=A4=BA?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 正確性: - 明示的な -s 0 / --scale=0 が無警告でフォールバックする穴を修正(P2)。 *scale == 0 センチネルは未指定と明示 0 を区別できないため、flag.Visit で フラグが実際に指定されたかを判定する scaleFlagSet を追加。明示された不正値 (0・負数・inf・nan・範囲外)を漏れなく起動時警告に載せる。README が 0 を 不正値として警告対象に挙げている契約とも整合。 - 警告文からライブ変更で陳腐化する実効値("using Nms instead")を除去(P3)。 警告はヘッダーに永続表示される一方、↑/↓ で m.scale は変わるため、実効値を 埋め込むと「using 5ms」表示の横でフッターが「RTT Scale 10ms」になる矛盾が 起きていた。文言を "using the configured or default scale" にし、実効値は 常に正しいフッターの "RTT Scale" 行に委ねる。 構造: - resolveScale を (float64, string) からスケール専用 float64 に戻し、警告生成を parseArgs へ移動。警告が実効値非依存になったため config ロード前に確定でき、 明示判定が可能な flag 層で一元的に行える。 - scaleWarning(cli) は有効範囲を config.MinScale/MaxScale から整形(継続)。 テスト: - TestResolveScale をスケール専用に戻す。 - TestParseArgsScaleWarning を新設(-s 0 明示・--scale=0・負数・inf・nan・範囲外・ 未指定・有効値の 8 ケース)。明示 0 が警告される P2 の回帰ガード。 make check 全通過(tidy-check/fmt-check/vet/lint/test)・test-race 通過。go.sum 不変。 --- cmd/deadman/main.go | 82 +++++++++++++++++++++++++--------------- cmd/deadman/main_test.go | 79 ++++++++++++++++++++++++-------------- 2 files changed, 102 insertions(+), 59 deletions(-) diff --git a/cmd/deadman/main.go b/cmd/deadman/main.go index d660439..ba43b31 100644 --- a/cmd/deadman/main.go +++ b/cmd/deadman/main.go @@ -22,36 +22,36 @@ import ( // describe); a plain `go install`/`go build` leaves it at "dev". var version = "dev" -// resolveScale picks the effective RTT-bar scale and, when an explicit -s value was -// rejected as unusable, an operator-facing warning ("" otherwise): an explicit CLI -s -// wins, else a config "scale" directive, else config.DefaultScale. 0 means "unset" for -// both inputs (the -s flag defaults to 0, and a missing or invalid "scale" directive -// leaves cfg.Scale at 0), so passing -s is never indistinguishable from its own default -// and a config scale can take effect. +// resolveScale picks the effective RTT-bar scale: an explicit, usable CLI -s wins, else a +// config "scale" directive, else config.DefaultScale. 0 means "unset" for both inputs (the +// -s flag defaults to 0, and a missing or invalid "scale" directive leaves cfg.Scale at 0), +// so a config scale can take effect when -s is absent. // -// Both inputs go through the same config.ValidScale predicate the "scale" directive -// uses, so a non-finite or out-of-range value (e.g. -s Inf, which flag.Float64 accepts) -// is rejected rather than flattening every bar; the rejection is not fatal — the value -// is dropped and the probe runs. Scale and warning are computed in one place so the -// message always names what is actually in effect — a valid config "scale" or the -// default — and cannot drift from the accept/reject decision. -func resolveScale(cli, cfg float64) (float64, string) { +// Both inputs go through the same config.ValidScale predicate the "scale" directive uses, +// so a non-finite or out-of-range value (e.g. -s Inf, which flag.Float64 accepts) is +// dropped rather than flattening every bar. The rejection is not fatal and is surfaced as +// a startup warning by parseArgs (which alone can tell an explicit -s 0 from an unset one). +func resolveScale(cli, cfg float64) float64 { if config.ValidScale(cli) { - return cli, "" + return cli } - scale := config.ScaleOrDefault(cfg) - - if cli == 0 { // unset -s: nothing to surface. - return scale, "" - } + return config.ScaleOrDefault(cfg) +} - return scale, fmt.Sprintf( - "-s %g ignored: not a usable RTT-bar scale (%s..%s ms); using %gms instead", +// scaleWarning builds the operator-facing warning for an explicitly-passed but unusable -s +// value (0, negative, inf, nan, or out of range). It names the rejected value and the +// usable window — formatted from config's bounds so the prose can't drift from the +// predicate — but deliberately NOT the effective scale: the warning is rendered +// persistently in the header while the live scale can still change (↑/↓), so embedding +// "using Nms" would go stale. The footer's "RTT Scale" line always shows the value in +// effect, so the operator can read the real scale there. +func scaleWarning(cli float64) string { + return fmt.Sprintf( + "-s %g ignored: not a usable RTT-bar scale (%s..%s ms); using the configured or default scale", cli, strconv.FormatFloat(config.MinScale, 'f', -1, 64), strconv.FormatFloat(config.MaxScale, 'f', -1, 64), - scale, ) } @@ -107,14 +107,39 @@ func parseArgs(args []string) (tui.Options, error) { ) } - return tui.Options{ + opts := tui.Options{ Async: *async, Blink: *blink, Scale: *scale, LogDir: *logdir, Cols: *cols, ConfigPath: positional[0], - }, nil + } + + // Surface an explicitly-passed but unusable -s as a startup warning. The value is still + // dropped and the configured/default scale is used (see resolveScale); this only makes + // the silently-ignored input visible. + if scaleFlagSet(fs) && !config.ValidScale(*scale) { + opts.Warnings = append(opts.Warnings, scaleWarning(*scale)) + } + + return opts, nil +} + +// scaleFlagSet reports whether -s or --scale was actually given on the command line. It +// distinguishes an explicit `-s 0` (a rejected value worth warning about) from an unset -s +// — the *scale == 0 sentinel cannot, since both leave it at the flag's 0 default — by +// asking the flag set which flags were set rather than inspecting the parsed value. +func scaleFlagSet(fs *flag.FlagSet) bool { + set := false + + fs.Visit(func(f *flag.Flag) { + if f.Name == "s" || f.Name == "scale" { + set = true + } + }) + + return set } // resolveCols picks the newspaper-column count: an explicit CLI -c/--split wins, @@ -157,17 +182,14 @@ func main() { os.Exit(1) } - scale, warn := resolveScale(opts.Scale, cfg.Scale) + // The invalid-scale warning (if any) was already queued by parseArgs, which alone can + // tell an explicit -s from its default; here we only resolve the effective value. opts.Columns = cfg.Columns - opts.Scale = scale + opts.Scale = resolveScale(opts.Scale, cfg.Scale) opts.Precision = cfg.Precision opts.Cols = resolveCols(opts.Cols, cfg.Cols) opts.Version = version - if warn != "" { - opts.Warnings = append(opts.Warnings, warn) - } - if opts.LogDir != "" { opts.LogWriter = monitor.NewLogWriter(opts.LogDir) } diff --git a/cmd/deadman/main_test.go b/cmd/deadman/main_test.go index a652b0b..d566e98 100644 --- a/cmd/deadman/main_test.go +++ b/cmd/deadman/main_test.go @@ -3,10 +3,8 @@ package main import ( "errors" "flag" - "fmt" "math" "os" - "strings" "testing" "github.com/yuu61/deadman/internal/config" @@ -124,47 +122,70 @@ func TestParseArgsRejectsMultipleConfigs(t *testing.T) { } } -// TestResolveScale pins both the effective scale and its companion warning: an explicitly -// passed, unusable -s warns (the value is dropped and the probe runs) while an unset or -// usable -s is silent, and a warning always names the scale actually in effect. +// TestResolveScale pins the effective scale: an explicit, usable CLI -s wins, else the +// config "scale", else the default; any unusable value (here passed as a bare float, not +// via the flag) is dropped rather than flattening every bar. The companion warning is the +// flag layer's job — it needs to tell an explicit -s from its default — so it is covered +// by TestParseArgsScaleWarning, not here. func TestResolveScale(t *testing.T) { cases := []struct { name string cli, cfg float64 want float64 - wantWarn bool }{ - {"cli explicit wins over config", 20, 5, 20, false}, - {"config used when cli unset", 0, 5, 5, false}, - {"cli used when config unset", 7, 0, 7, false}, - {"sub-ms cli is honored", 0.5, 0, 0.5, false}, - {"non-finite cli falls back to config and warns", math.Inf(1), 5, 5, true}, - {"nan cli falls back to config and warns", math.NaN(), 5, 5, true}, - {"negative cli falls back to default and warns", -5, 0, config.DefaultScale, true}, - {"out-of-range cli falls back to default and warns", 1e-300, 0, config.DefaultScale, true}, - {"non-finite cfg falls back to default", 0, math.Inf(1), config.DefaultScale, false}, - {"nan cfg falls back to default", 0, math.NaN(), config.DefaultScale, false}, - {"default when both unset", 0, 0, config.DefaultScale, false}, + {"cli explicit wins over config", 20, 5, 20}, + {"config used when cli unset", 0, 5, 5}, + {"cli used when config unset", 7, 0, 7}, + {"sub-ms cli is honored", 0.5, 0, 0.5}, + {"non-finite cli falls back to config", math.Inf(1), 5, 5}, + {"nan cli falls back to config", math.NaN(), 5, 5}, + {"out-of-range cli falls back to default", 1e-300, 0, config.DefaultScale}, + {"non-finite cfg falls back to default", 0, math.Inf(1), config.DefaultScale}, + {"nan cfg falls back to default", 0, math.NaN(), config.DefaultScale}, + {"default when both unset", 0, 0, config.DefaultScale}, } for _, c := range cases { t.Run(c.name, func(t *testing.T) { - got, warn := resolveScale(c.cli, c.cfg) - if got != c.want { + if got := resolveScale(c.cli, c.cfg); got != c.want { t.Errorf("resolveScale(%g, %g) = %g, want %g", c.cli, c.cfg, got, c.want) } + }) + } +} - if (warn != "") != c.wantWarn { - t.Errorf( - "resolveScale(%g, %g) warning = %q, wantWarn=%v", - c.cli, c.cfg, warn, c.wantWarn, - ) +// TestParseArgsScaleWarning checks that an explicitly-passed, unusable -s surfaces a +// startup warning while an unset or usable -s stays silent. The explicit `-s 0` case is +// the one a value sentinel cannot catch (an unset -s also parses to 0): fs.Visit +// distinguishes them, so the rejected zero is not silently dropped. The warning omits the +// effective scale on purpose (it is rendered persistently while ↑/↓ can change the live +// scale), so this only asserts presence/absence, not an embedded "using Nms". +func TestParseArgsScaleWarning(t *testing.T) { + cases := []struct { + name string + args []string + wantWarn bool + }{ + {"unset -s is silent", []string{"deadman.conf"}, false}, + {"usable -s is silent", []string{"-s", "5", "deadman.conf"}, false}, + {"explicit zero -s warns", []string{"-s", "0", "deadman.conf"}, true}, + {"explicit zero --scale warns", []string{"--scale=0", "deadman.conf"}, true}, + {"negative -s warns", []string{"-s", "-5", "deadman.conf"}, true}, + {"inf -s warns", []string{"-s", "inf", "deadman.conf"}, true}, + {"nan -s warns", []string{"-s", "nan", "deadman.conf"}, true}, + {"out-of-range -s warns", []string{"-s", "1e-300", "deadman.conf"}, true}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + opts, err := parseArgs(c.args) + if err != nil { + t.Fatal(err) } - // The warning must name the scale actually in effect (config or default), - // not assume the default. - if effective := fmt.Sprintf("using %gms", c.want); c.wantWarn && - !strings.Contains(warn, effective) { - t.Errorf("warning %q should contain %q", warn, effective) + if gotWarn := len(opts.Warnings) > 0; gotWarn != c.wantWarn { + t.Errorf( + "parseArgs(%v) warnings = %v, wantWarn=%v", + c.args, opts.Warnings, c.wantWarn, + ) } }) } From 10efe92151a492551dc1a126a7039899f80d3e07 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=B5=90=E5=8F=8B?= Date: Sat, 13 Jun 2026 12:17:55 +0900 Subject: [PATCH 9/9] =?UTF-8?q?test:=20scale=20=E3=83=87=E3=82=A3=E3=83=AC?= =?UTF-8?q?=E3=82=AF=E3=83=86=E3=82=A3=E3=83=96=E3=81=AE=20ValidScale=20?= =?UTF-8?q?=E9=85=8D=E7=B7=9A=E3=81=AB=E5=9B=9E=E5=B8=B0=E3=82=AC=E3=83=BC?= =?UTF-8?q?=E3=83=89=E8=BF=BD=E5=8A=A0=E3=83=BB=E8=AD=A6=E5=91=8A=E3=81=AE?= =?UTF-8?q?=E8=A1=A8=E8=A8=98=E5=B7=AE=E3=82=92=E6=98=8E=E6=96=87=E5=8C=96?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - config_test: TestParseConfigScaleLenient に有限・正・範囲外の scale 1e-300/1e300 を追加。IsInf/IsNaN ガード+素の n>0 への退行を捕捉する(nan 単独では捕捉できず、 inf は符号でしか効かない=有限の大小を検証できるのはこの2ケースのみ) - main: scaleWarning が拒否値を %g・境界を 'f' で出す使い分けを設計判断としてコメント化。 'f' 統一は -s 1e-300 でラベルが ~300 字に膨張し MinScale の趣旨に反するため採らない --- cmd/deadman/main.go | 6 ++++++ internal/config/config_test.go | 14 ++++++++++---- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/cmd/deadman/main.go b/cmd/deadman/main.go index ba43b31..5a062dd 100644 --- a/cmd/deadman/main.go +++ b/cmd/deadman/main.go @@ -46,6 +46,12 @@ func resolveScale(cli, cfg float64) float64 { // persistently in the header while the live scale can still change (↑/↓), so embedding // "using Nms" would go stale. The footer's "RTT Scale" line always shows the value in // effect, so the operator can read the real scale there. +// +// The rejected value uses %g while the bounds use FormatFloat 'f' on purpose, not by +// oversight: the value is arbitrary operator input that may be extreme, and %g keeps +// -s 1e-300 a compact "1e-300" rather than the ~300-digit decimal 'f' would emit (the +// very label ballooning MinScale exists to reject); the bounds are known round numbers +// that read cleanest in plain 'f' (0.0001..1000000, no "1e+06"). func scaleWarning(cli float64) string { return fmt.Sprintf( "-s %g ignored: not a usable RTT-bar scale (%s..%s ms); using the configured or default scale", diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 8ef6a23..beeee5d 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -498,12 +498,18 @@ func TestParseConfigSplitLenient(t *testing.T) { } func TestParseConfigScaleLenient(t *testing.T) { - // A non-numeric, non-positive, or non-finite scale is ignored, leaving Scale unset - // (0) so the caller falls back to the CLI/default rather than aborting the parse. - // All of these fall to the single ValidScale range predicate: ParseFloat accepts - // "inf"/"nan", but +Inf fails v <= MaxScale and NaN fails both range comparisons. + // A non-numeric, non-positive, non-finite, or out-of-range finite scale is ignored, + // leaving Scale unset (0) so the caller falls back to the CLI/default rather than + // aborting the parse. All of these fall to the single ValidScale range predicate: + // ParseFloat accepts "inf"/"nan", but +Inf fails v <= MaxScale and NaN fails both + // range comparisons. The "1e-300"/"1e300" cases pin the directive's finite range + // bounds (MinScale/MaxScale) specifically: they are finite and positive, so a + // regression that rejected only the non-finite inputs but kept a bare n>0 (e.g. an + // explicit IsInf/IsNaN guard plus n>0) would accept them and flatten every bar — the + // nan case alone can't catch that, and inf only catches it by sign, not by magnitude. for _, in := range []string{ "scale abc\n", "scale -3\n", "scale 0\n", "scale\n", "scale inf\n", "scale nan\n", + "scale 1e-300\n", "scale 1e300\n", } { cfg, err := ParseConfig(strings.NewReader(in)) if err != nil {