fix: parse-failure tracking, Windows tee path, passthrough output loss (#3339) - #3344
Open
SomSamantray wants to merge 4 commits into
Open
fix: parse-failure tracking, Windows tee path, passthrough output loss (#3339)#3344SomSamantray wants to merge 4 commits into
SomSamantray wants to merge 4 commits into
Conversation
added 4 commits
July 31, 2026 19:20
…loss When PlaywrightParser::parse falls back to Passthrough (all parsing tiers failed), the run was tracked through the same savings timer used for successful parses -- so the truncation-driven size difference was recorded as token savings even though nothing was parsed. Route genuine Tier-3 failures through the existing record_parse_failure_silent + track_passthrough plumbing instead. Also build the passthrough fallback content from the combined stdout+stderr rather than the parser's stdout-only truncation, so a failure reported only on stderr (empty stdout) isn't silently dropped. Part of rtk-ai#3339.
Same fix as playwright applied to pnpm list and pnpm outdated: genuine Tier-3 parse failures now route through record_parse_failure_silent + track_passthrough instead of being counted as token savings, and the passthrough fallback content is built from the combined stdout+stderr so a stderr-only failure isn't dropped. pnpm list's Tier-1/Degraded guard and tracking stay anchored to stdout alone (unchanged from before this fix) -- only the Tier-3 arm uses the combined stream, so a stderr notice alongside a successful parse doesn't inflate the reported savings ratio. pnpm outdated's existing "All packages up-to-date" empty-display fallback is untouched. Part of rtk-ai#3339.
Same fix as playwright/pnpm applied to vitest/jest run. Adds an is_parse_failure field to FormattedTestOutput, set only in the genuine ParseResult::Passthrough arm inside format_test_output -- distinct from the explicit user-requested passthrough path (an explicit --reporter override), which is not a failure and keeps its existing behavior. run_test branches its tracking call on that field: genuine failures route through record_parse_failure_silent + track_passthrough; everything else keeps the existing timer.track call. The passthrough fallback content is now built from the combined stdout+stderr instead of stdout alone. Part of rtk-ai#3339.
…hell's HOME The tee recovery hint (e.g. [full output: ~/path/to/log]) mixed a forward-slash ~ with native backslash separators on Windows, and could resolve ~ against a different home directory than the invoking Git Bash/MSYS shell's own ~ expansion uses. - Normalize the home-relative portion to forward slashes unconditionally in display_path, not only inside the whitespace-quoting branch that already did this for paths with spaces. - Prefer the HOME environment variable over the OS home-directory API when stripping the home prefix, since Git Bash/MSYS set HOME themselves and that is the root their own ~ expansion resolves against. HOME is commonly MSYS-style (/c/Users/name); convert it to a native path (C:\Users\name) before comparing, since passed through unconverted it would never match a native path via strip_prefix and would silently defeat the ~ shorthand entirely. Reject an empty HOME value, since an empty PathBuf's strip_prefix trivially matches any path. Part of rtk-ai#3339.
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the three related bugs reported in #3339, all rooted in how rtk's Tier-3 "parsing failed" fallback (
ParseResult::Passthrough) is tracked and displayed.1.
rtk gaincounted parse failures as savingstimer.track(...)was called unconditionally, even for Passthrough-tier runs. Since the passthrough content is a truncated view of the raw output, the size difference between "raw" and "shown" was counted as savings — even though nothing was parsed or summarized, and truncation only happened because parsing failed.Fix: route genuine Tier-3 failures through the existing
record_parse_failure_silent+TimedExecution::track_passthroughplumbing (already used byrtk gain --failures) instead of the generic savings timer. Applied toplaywright test,pnpm list,pnpm outdated, andvitest/jest run— the four call sites sharing this pattern. Explicit user-requested passthrough (e.g. vitest's--reporteroverride) is untouched — it isn't a failure.2. Tee hint path broken on Windows
[full output: ~/path/to/log]mixed a forward-slash~with native backslash separators, and could resolve~against a different home directory than the invoking Git Bash/MSYS shell's own~expansion uses.Fix, in
display_path:$HOMEover the OS home-directory API when stripping the home prefix, converting Git Bash/MSYS's POSIX-styleHOME(/c/Users/name) to a native path first — otherwise it would never match viastrip_prefixand would silently defeat the~shorthand.HOMEvalue (an emptyPathBuf'sstrip_prefixtrivially matches everything).This keeps the existing
~/shorthand (per #2325's direction) rather than reverting to always-absolute paths, an approach previously tried in #2187/#2701.3. Passthrough could silently emit nothing
When parsing failed and a command's real output was empty on stdout (diagnostic on stderr instead), the passthrough fallback was itself empty.
never_worsethen preferred that empty result over the real combined output, discarding it.Fix: build the passthrough fallback content from the combined stdout+stderr instead of stdout alone, for all four call sites above.
Notes
developclone before implementing.cargo fmt --all,cargo clippy --all-targets -- -D warnings, andcargo test --allall pass (2523 tests; the only 2 failures in this environment —hooks::rewrite_cmd::tests::unattestable_passthrough::*— are pre-existing and unrelated, reproduced identically on unmodifieddevelop).pnpm list's Tier-1/Degraded guard and tracking stay anchored to stdout alone (unchanged) — only the Tier-3 arm uses the combined stream, so a stderr notice alongside a successful parse doesn't inflate the reported savings ratio.