Skip to content

fix(playwright): fix JSON parser to match real Playwright output format - #193

Merged
pszymkowiak merged 1 commit into
rtk-ai:masterfrom
USerik:fix/playwright-json-parser
Mar 4, 2026
Merged

fix(playwright): fix JSON parser to match real Playwright output format#193
pszymkowiak merged 1 commit into
rtk-ai:masterfrom
USerik:fix/playwright-json-parser

Conversation

@USerik

@USerik USerik commented Feb 18, 2026

Copy link
Copy Markdown
Contributor

Three bugs caused rtk playwright test to always fail with EXIT: 1:

  1. Wrong argument order: --reporter=json was inserted before the subcommand (e.g. playwright --reporter=json test), but Playwright requires flags after the subcommand (playwright test --reporter=json).

  2. Float duration deserialization: PlaywrightStats.duration was typed as u64, but real Playwright output emits a float (e.g. 3519.703...), causing serde_json to fail with a type error.

  3. Wrong suite structure: Suites contain specs (not tests). Each spec has ok: bool and tests (per-browser executions). The old struct used tests: Vec<PlaywrightTest> directly in suites, which never matched real output and always failed JSON parsing.

Fixes:

  • Move --reporter=json to after the first arg (the subcommand)
  • Change duration: u64 to duration: f64 in PlaywrightStats
  • Rewrite PlaywrightSuite → specs: Vec<PlaywrightSpec>
  • Add PlaywrightSpec with ok: bool and tests: Vec<PlaywrightExecution>
  • Add PlaywrightExecution with status and results: Vec<PlaywrightAttempt>
  • Update PlaywrightAttempt to use errors: Vec<PlaywrightError> (array)
  • Add tests covering float duration, real suite structure, and failure details

@pszymkowiak

pszymkowiak commented Feb 20, 2026

Copy link
Copy Markdown
Collaborator

Thanks for this! Could you rebase on master? The branch is 2 commits behind. We'll review once it's up to date.

@devkeruse @claude
fix(playwright): fix JSON parser to match real Playwright output format
8f03e7e 
Three bugs caused `rtk playwright test` to always fail with EXIT: 1:

1. **Wrong argument order**: `--reporter=json` was inserted before the
   subcommand (e.g. `playwright --reporter=json test`), but Playwright
   requires flags after the subcommand (`playwright test --reporter=json`).

2. **Float duration deserialization**: `PlaywrightStats.duration` was typed
   as `u64`, but real Playwright output emits a float (e.g. `3519.703...`),
   causing serde_json to fail with a type error.

3. **Wrong suite structure**: Suites contain `specs` (not `tests`). Each
   spec has `ok: bool` and `tests` (per-browser executions). The old struct
   used `tests: Vec<PlaywrightTest>` directly in suites, which never matched
   real output and always failed JSON parsing.

Fixes:
- Move `--reporter=json` to after the first arg (the subcommand)
- Change `duration: u64` to `duration: f64` in `PlaywrightStats`
- Rewrite `PlaywrightSuite` → `specs: Vec<PlaywrightSpec>`
- Add `PlaywrightSpec` with `ok: bool` and `tests: Vec<PlaywrightExecution>`
- Add `PlaywrightExecution` with `status` and `results: Vec<PlaywrightAttempt>`
- Update `PlaywrightAttempt` to use `errors: Vec<PlaywrightError>` (array)
- Add tests covering float duration, real suite structure, and failure details

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@USerik
USerik force-pushed the fix/playwright-json-parser branch from c963263 to 8f03e7e Compare February 25, 2026 07:23
heAdz0r added a commit to heAdz0r/rtk that referenced this pull request Feb 28, 2026
@heAdz0r @claude
release: fork.16 — backport 8 upstream PRs (rtk-ai#246, rtk-ai#193, r…
53472b0 
…tk-ai#217, rtk-ai#196, rtk-ai#248, rtk-ai#211, rtk-ai#200, rtk-ai#192, rtk-ai#268)

Wave 1 (critical bugs):
- fix(registry): fi/done moved to IGNORED_EXACT — find no longer shadowed (rtk-ai#246)
- fix(playwright): f64 duration, specs[] structure, --reporter=json after subcmd (rtk-ai#193)
- fix(gh): should_passthrough_gh_view for --json/--jq/--template/--web in view_pr/issue/run (rtk-ai#217+196)

Wave 2 (reliability):
- fix(git): is_blob_show_arg — blob show passthrough without trailing-newline trim (rtk-ai#248)
- fix(find): parse_find_args with native -name/-type/-maxdepth/-iname support (rtk-ai#211)
- fix(main): graceful Clap fallback + parse_failures SQLite table + rtk gain --failures (rtk-ai#200)

Wave 3 (UX):
- feat(git): global options -C/-c/--git-dir/--work-tree/--no-pager/--no-optional-locks/--bare/--literal-pathspecs (rtk-ai#192)
- feat(proxy): streaming output via spawn()+threads instead of buffered output() (rtk-ai#268)

Tests: 1091 → 1117 (+26), 0 regressions

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@pszymkowiak

pszymkowiak commented Mar 2, 2026

Copy link
Copy Markdown
Collaborator

Hi, this PR has conflicts with master in src/playwright_cmd.rs. Could you rebase on current master? Thanks!

@pszymkowiak

pszymkowiak commented Mar 4, 2026

Copy link
Copy Markdown
Collaborator

Reviewed and tested locally — 414 tests pass (5 playwright-specific), 0 clippy warnings. Clean merge with master.

All 3 bugs are real issues that made rtk playwright test unusable:

  1. ✅ --reporter=json position fixed (after subcommand)
  2. ✅ duration: f64 instead of u64
  3. ✅ Correct suite structure (specs → tests → results)

LGTM — ready to merge.

@pszymkowiak
pszymkowiak merged commit 4eb6cf4 into rtk-ai:master Mar 4, 2026
@github-actions github-actions Bot mentioned this pull request Mar 4, 2026
Merged
@BrewTestBot BrewTestBot mentioned this pull request Mar 4, 2026
Merged
maxkulish added a commit to maxkulish/rtk that referenced this pull request Mar 18, 2026
@maxkulish
fix: port P1.3 other module fixes from upstream
bacd6c0 
Implements 6 bug fixes from upstream rtk-ai/rtk (v0.29.0):

P1.3a - playwright: JSON parser already matches real output format (rtk-ai#193)
- Already uses proper suites → specs → tests → results structure
- Handles float duration correctly (f64 type)
- No changes needed

P1.3b - cargo: `--` separator already preserved (rtk-ai#326)
- Already uses trailing_var_arg + allow_hyphen_values
- rtk cargo test -- --nocapture works correctly
- No changes needed

P1.3c - lint: strip npx/bunx/pnpm prefixes for linter detection (rtk-ai#186/rtk-ai#366)
- Add strip_pm_prefix() to remove package manager prefixes
- Add detect_linter() for accurate linter identification
- Fixes: npx eslint, bunx biome, pnpm exec eslint
- Tests: test_strip_pm_prefix(), test_detect_linter()

P1.3d - grep: BRE alternation translation already implemented (rtk-ai#206)
- Already translates \| (BRE) → | (PCRE) for rg
- Already strips -r flag (rg recursive by default)
- Comprehensive tests already exist
- No changes needed

P1.3e - filter: add Language::Data to prevent JSON/YAML/TOML corruption (rtk-ai#464/rtk-ai#479)
- Add Language::Data variant for data files
- Map 20 data file extensions (json, yaml, toml, xml, md, csv, etc.)
- Data language has NO comment patterns (all None)
- Fixes: packages/* in package.json no longer stripped as /* comment
- Tests: test_data_language_no_comment_stripping()

P1.3f - npm: fix routing to distinguish subcommands from scripts (rtk-ai#470)
- Add NPM_SUBCOMMANDS constant (59 known npm commands)
- Proper routing: npm install → npm install (not npm run install)
- Proper routing: npm build → npm run build (script)
- Tests: test_npm_subcommand_routing() validates all 59 subcommands

Files modified:
- src/lint_cmd.rs: Add PM prefix stripping
- src/filter.rs: Add Language::Data variant
- src/local_llm.rs: Add Language::Data match arm
- src/npm_cmd.rs: Add NPM_SUBCOMMANDS routing

Quality checks:
✅ cargo fmt --all
✅ cargo clippy --all-targets (0 errors)
✅ cargo test (514 passed; 0 failed)

Improves RTK reliability across JS/TS, data files, and npm workflows.
thehoff pushed a commit to thehoff/contextcrawler that referenced this pull request May 14, 2026
@USerik @devkeruse @claude
fix(playwright): fix JSON parser to match real Playwright output form…
b46e9b1 
…at (rtk-ai#193)

Three bugs caused `rtk playwright test` to always fail with EXIT: 1:

1. **Wrong argument order**: `--reporter=json` was inserted before the
   subcommand (e.g. `playwright --reporter=json test`), but Playwright
   requires flags after the subcommand (`playwright test --reporter=json`).

2. **Float duration deserialization**: `PlaywrightStats.duration` was typed
   as `u64`, but real Playwright output emits a float (e.g. `3519.703...`),
   causing serde_json to fail with a type error.

3. **Wrong suite structure**: Suites contain `specs` (not `tests`). Each
   spec has `ok: bool` and `tests` (per-browser executions). The old struct
   used `tests: Vec<PlaywrightTest>` directly in suites, which never matched
   real output and always failed JSON parsing.

Fixes:
- Move `--reporter=json` to after the first arg (the subcommand)
- Change `duration: u64` to `duration: f64` in `PlaywrightStats`
- Rewrite `PlaywrightSuite` → `specs: Vec<PlaywrightSpec>`
- Add `PlaywrightSpec` with `ok: bool` and `tests: Vec<PlaywrightExecution>`
- Add `PlaywrightExecution` with `status` and `results: Vec<PlaywrightAttempt>`
- Update `PlaywrightAttempt` to use `errors: Vec<PlaywrightError>` (array)
- Add tests covering float duration, real suite structure, and failure details

Co-authored-by: devkeruse <devkeruse@gmail.com>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
thehoff added a commit to thehoff/contextcrawler that referenced this pull request Jun 5, 2026
@thehoff @claude
perf(grep): filter recursive context grep instead of raw passthrough
f35f00e 
Issue rtk-ai#193: grep is ~1/3 of command traffic but averaged ~2.6% token
savings because flagged/recursive calls fell through to raw passthrough
(0 savings). PR #127 fixed the clap parse errors for standard grep flags,
but context-flag calls (-A/-B/-C) then routed to run_grep_format_passthrough,
which inherits stdout straight to the terminal — no filtering at all. This
was the single highest-yield grep shape (861 of 1629 fallbacks used -r,
most paired with context flags).

Change:
- Add grep_cmd::filter_context_output: parses rg/grep context output by
  splitting on the `--` group separator, keeps each match-group intact
  (context lines + the `file-line-` vs `file:line:` format are preserved),
  and caps groups per-file (grep_max_per_file=25) and globally
  (grep_max_results=200) using the existing [limits] config. Truncation is
  always signalled with a "[+N more groups]" marker so matches are never
  dropped silently.
- Add run_grep_context_filtered in main.rs: for the Passthrough variant,
  route true context-flag calls (has_grep_context_flag) through rg with
  CAPTURED output, filter it, and apply the no_bloat guard so a small
  result is never inflated by the framing. -H/value-flag calls (no context
  flag) stay on the raw passthrough — single-line output, nothing to group.
- Fallback discipline: if rg is absent or capture fails, fall back to the
  raw passthrough so the user always gets their matches.

Council/Codex review fix: group_file now buckets the per-file cap on the
RAW path, not the compacted display form. The old code keyed per_file_seen
on compact_path(...), so two distinct files that collapse to the same short
display string shared one counter and wrongly truncated each other (the
emitted group text uses raw paths, so the compaction served no display
purpose here). compact_path stays for display in the non-context filter.

Measured on a real recursive context grep (grep -rn -C1 let src/hooks/):
6497 -> 966 lines, 85.1% token savings (29095 -> 4348 tokens). Previously
this path yielded 0% (raw passthrough).

Tests (tests/fixtures/grep_context_raw.txt is real `grep -rn -C1` output):
- grouping + per-file cap, global cap, empty input, file attribution
- golden-form snapshot (repo has no insta dep; deterministic assertions)
- token-savings test asserting >=60% on the recursive fixture
- raw-path bucketing: distinct long files that compact to the same display
  form get separate per-file counters and do not truncate each other
All 99 grep tests pass; no new clippy warnings. The 7 pre-existing
hooks::integrity failures are environmental (/tmp mode 775 in sandbox) and
fail identically on the unmodified tree.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
thehoff added a commit to thehoff/contextcrawler that referenced this pull request Jun 5, 2026
@thehoff @claude
fix(grep): parse single-file output containing colons (upstream be226d4)
7ed8bb2 
A single-file grep/rg search emits `<line>:<content>` (no filename),
which the old `splitn(3, ':')` length heuristic could not distinguish
from `<file>:<line>:<content>` once the matched line content contained
a colon (`Foo::bar()`, `http://...`, `std::vec`). The parser mis-split
those lines into phantom `[file] N` buckets, feeding wrong output to the
agent (upstream issue rtk-ai#1436 / rtk-ai#1554).

Adapt the upstream NUL-separation approach (the fork's grep is an
independent rewrite, issue rtk-ai#193, so the idea is ported, not the diff):

- Force the filename to always be present and unambiguously delimited:
  rg is invoked with `-H --null` and the grep fallback with `-rnHZ`, so
  every non-context match line is `file\0line:content`. NUL can never
  appear in a path, line number, or content, so the file boundary is
  exact even for Windows drive letters or filenames containing
  `:digits:`.
- Replace the fragile `splitn(3, ':')` heuristic with a lazy_static
  regex `^([^\x00]+)\x00(\d+):(.*)$` in a new `parse_match_line` helper
  that borrows the content slice (no allocation).

The NUL form is used ONLY for parsing — it must never reach stdout. On
small results `no_bloat` may pick the raw capture as the smaller form, so
the raw baseline and any raw passthrough use a reconstructed display form
`raw_output.replace('\0', ":")` (the normal `file:line:content` a real
grep shows). Both no_bloat call sites and their savings-tracking
baselines use this display form; parsing still consumes the NUL capture.

Scope: only the non-context `grep_cmd::run` path changes. The rtk-ai#193
context filter (`filter_context_output` / `group_file`, splitting on
`\n--\n` with `file:line:` / `file-line-` separators) is untouched —
context-flag calls route through main.rs's `run_grep_context_filtered`
and never see `--null`/`-Z`.

Tests: 8 new cases covering `::`/URL/Rust-path content parsing as one
intact match, filenames containing `:digits:`, empty content, malformed
lines returning None, end-to-end NUL-separated multi-file grouping with
no phantom buckets, and a regression asserting the emitted output never
contains a NUL byte on the no_bloat-picks-raw path. grep_cmd suite all
green.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

awaiting-changes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@USerik @pszymkowiak @devkeruse