fix(filters): aggresivity batch fix#1895
Conversation
…iltering Several filters dropped or rewrote information an agent needs, producing output that was misleading rather than merely compressed. This fixes the clear-cut cases surfaced by the TheDecipherist/rtk-test suite: - ls: stop hiding the `.env` file (removed `.env`/`env` from NOISE_DIRS). - git status: restore the explicit "HEAD detached at <sha>" line instead of the opaque "* HEAD (no branch)". - log/docker logs: recognise CRITICAL/FATAL/ALERT/EMERGENCY/SEVERE (and NOTICE) as severities so those lines are no longer silently filtered; expose `log` as a `rtk pipe` filter and accept a positional filter name. - wc: forward stdin to the child so `cat file | rtk wc` counts the pipe instead of reporting 0 (new RunOptions::inherit_stdin). - jest/vitest: include skipped/pending test count in the compact summary. - pytest: surface xfailed/xpassed counts and list XFAIL/XPASS entries with their reasons (adds `-rxX`). - ruff/eslint: list individual violations with file:line:col, not just rule/file group counts. - pnpm/npm list: render the actual package list instead of the false-positive "All packages up-to-date" when there's no upgrade info. - git add: stay silent on a no-op instead of printing an ambiguous "ok". - git stash: pass git's own message through instead of collapsing to "ok". - docker ps: keep the Status column (health/restart info) and list stopped/exited containers; docker images: show all images with full names; docker compose ps: use `-a` so crashed services stay visible. All 1870 unit tests pass; cargo fmt + clippy clean.
KuSh
left a comment
There was a problem hiding this comment.
Minor nitpicks and questions. Otherwise LGTM!
| } | ||
| } | ||
| if running.len() > 20 { | ||
| rtk.push_str(&format!(" ... +{} more\n", running.len() - 20)); |
There was a problem hiding this comment.
nitpick, prefer utf8 character …
| rtk.push_str(&format!(" ... +{} more\n", running.len() - 20)); | |
| rtk.push_str(&format!(" … +{} more\n", running.len() - 20)); |
| if count > 15 { | ||
| rtk.push_str(&format!(" ... +{} more", count - 15)); | ||
| if stopped.len() > 20 { | ||
| rtk.push_str(&format!(" ... +{} more\n", stopped.len() - 20)); |
There was a problem hiding this comment.
| rtk.push_str(&format!(" ... +{} more\n", stopped.len() - 20)); | |
| rtk.push_str(&format!(" … +{} more\n", stopped.len() - 20)); |
| if lines.len() > 15 { | ||
| rtk.push_str(&format!(" ... +{} more", lines.len() - 15)); | ||
| if lines.len() > MAX_IMAGES { | ||
| rtk.push_str(&format!(" ... +{} more\n", lines.len() - MAX_IMAGES)); |
There was a problem hiding this comment.
| rtk.push_str(&format!(" ... +{} more\n", lines.len() - MAX_IMAGES)); | |
| rtk.push_str(&format!(" … +{} more\n", lines.len() - MAX_IMAGES)); |
|
|
||
| // Porcelain `-b` reduces a detached HEAD to "## HEAD (no branch)"; restore | ||
| // the explicit "HEAD detached at <sha>" from the plain status we captured. | ||
| if let Some(detached) = extract_detached_head(&raw_output) { |
There was a problem hiding this comment.
Wouldn't it be better to check that before calling format_status_output and pass it detached instead of replacing the result afterward?
| if pkgs.len() > 10 { | ||
| result.push_str(&format!(" ... +{} more\n", pkgs.len() - 10)); | ||
| if pkgs.len() > MAX_PER_LETTER { | ||
| result.push_str(&format!(" ... +{} more\n", pkgs.len() - MAX_PER_LETTER)); |
There was a problem hiding this comment.
while you're at it:
| result.push_str(&format!(" ... +{} more\n", pkgs.len() - MAX_PER_LETTER)); | |
| result.push_str(&format!(" … +{} more\n", pkgs.len() - MAX_PER_LETTER)); |
| } | ||
| if diagnostics.len() > MAX_VIOLATIONS { | ||
| result.push_str(&format!( | ||
| " ... +{} more violations\n", |
There was a problem hiding this comment.
| " ... +{} more violations\n", | |
| " … +{} more violations\n", |
| lines.push(format!(" {} {}{}", dep.name, dep.current_version, dev)); | ||
| } | ||
| if self.dependencies.len() > 50 { | ||
| lines.push(format!(" ... +{} more", self.dependencies.len() - 50)); |
There was a problem hiding this comment.
| lines.push(format!(" ... +{} more", self.dependencies.len() - 50)); | |
| lines.push(format!(" … +{} more", self.dependencies.len() - 50)); |
| for dep in self.dependencies.iter().take(50) { | ||
| let dev = if dep.dev_dependency { " (dev)" } else { "" }; | ||
| lines.push(format!(" {} {}{}", dep.name, dep.current_version, dev)); | ||
| } |
There was a problem hiding this comment.
Isn't there a hierarchy in deps? dep > dev dep > opt dep > ... shouldn't we first print dependencies, then if < 50 pick dev dependencies until we reach 50?
|
Tested the branch build against real commands — solid work, the filters no longer hide crashed containers, skipped/xfail tests, ruff locations, or report a false 1.
This is the tip of a larger issue: 2. Everything else is good to go. |
- fix signal truncation gaps + kush reviews - added a new tee and hints function to give an hint with tail (avoid re-ingest of the head) - extracted patterns in constants
- docker ps reverts to plain (no -a augmentation) - docker ps -a / compose ps -a wired to the running/stopped split filter - ruff Violations: capped at 50 + force_tee_tail_hint recovery - tests for the ruff and pytest xfail caps
Summary