Use eq_ignore_ascii_case in exit_watch level rank (DSK-129)#170
Conversation
Avoid per-line String allocation when ranking LogLevel::Other labels for --exit-on-level. Co-authored-by: Cursor <cursoragent@cursor.com>
📝 WalkthroughWalkthroughexit_watch.rs 内の event_level_rank 関数において、LogLevel::Other(s) のランク判定ロジックが変更されました。従来は文字列を小文字化してから match で別名判定していましたが、eq_ignore_ascii_case を用いた if/else チェーンに置き換えられています。ランクの対応関係（error/err/fatal=4, warn/warning=3, info=2, debug=1, trace=0, 未知値=2）自体は変更されていません。 Changes
Possibly related issues
重大度: 低。ロジック上の同値変換であり、判定結果（ランク値）に変化はありません。ただし、非同期処理・K8s連携・セキュリティへの影響は本変更には見当たりません。 🚥 Pre-merge checks | ❌ 2❌ Failed checks (2 warnings)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/rustern-core/src/pipeline/exit_watch.rs`:
- Around line 51-66: `exit_watch.rs` contains the same alias-to-rank if/else
chain as `level_classify.rs::classify_str`, so update the logic to share a
single helper or common classification function instead of duplicating the alias
list and priority order. Use the existing `classify_str`-style symbols as the
source of truth and have `exit_watch` call that shared path, or if this refactor
is out of scope for this PR, leave a clear follow-up rather than introducing
another copy.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 31da2962-b65e-4580-840c-b7ef30722ab3
📒 Files selected for processing (1)
crates/rustern-core/src/pipeline/exit_watch.rs
| if s.eq_ignore_ascii_case("error") | ||
| || s.eq_ignore_ascii_case("err") | ||
| || s.eq_ignore_ascii_case("fatal") | ||
| { | ||
| 4 | ||
| } else if s.eq_ignore_ascii_case("warn") || s.eq_ignore_ascii_case("warning") { | ||
| 3 | ||
| } else if s.eq_ignore_ascii_case("info") { | ||
| 2 | ||
| } else if s.eq_ignore_ascii_case("debug") { | ||
| 1 | ||
| } else if s.eq_ignore_ascii_case("trace") { | ||
| 0 | ||
| } else { | ||
| 2 | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoff
エイリアス判定ロジックの重複（level_classify.rs::classify_str と同一パターン）
このif/elseチェーンは level_classify.rs の classify_str とエイリアス集合・判定順序が完全に一致している。将来エイリアスを追加/変更する際、片方のみ更新して不整合を起こすリスクがある。共通ヘルパー（例: fn level_alias_rank(s: &str) -> Option<u8> あるいは共通の分類関数）に抽出することを検討する余地あり。ただし本PRの目的（アロケーション削減）を超える範囲のリファクタとなるため、別PRでの対応を推奨。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/rustern-core/src/pipeline/exit_watch.rs` around lines 51 - 66,
`exit_watch.rs` contains the same alias-to-rank if/else chain as
`level_classify.rs::classify_str`, so update the logic to share a single helper
or common classification function instead of duplicating the alias list and
priority order. Use the existing `classify_str`-style symbols as the source of
truth and have `exit_watch` call that shared path, or if this refactor is out of
scope for this PR, leave a clear follow-up rather than introducing another copy.
1 participant
Summary
to_ascii_lowercase()inevent_level_rankwitheq_ignore_ascii_caseforLogLevel::Other.Test plan
cargo test -p rustern-core --lib pipeline::exit_watchMade with Cursor
Summary by CodeRabbit
"error" / "err" / "fatal"、"warn" / "warning"、"info"、"debug"、"trace"の扱いが整理され、未知の値は既定のレベルにフォールバックします。