Repository navigation
fix: allow tee mode to record recall stats (fixes #4219) - #1
Open
TechWizard9999 wants to merge 1605 commits into
Open
TechWizard9999 wants to merge 1605 commits into
TechWizard9999 wants to merge 1605 commits into
Conversation
- core/runner.rs: bun and deno share a buffered engine and differ only in an open, prove, close policy table, so a guard cannot land on one runtime alone - core/runner.rs: deno gains the proof rule it never had, so a passing test that logs its own error line no longer reports as a failed suite - core/runner.rs: a block closes when the next one opens, so consecutive stack-free failures stay separate - tests: every scenario is asserted on both runtimes from real fixtures
- utils.rs: tool_exec prefers an explicitly named package runner over lockfile detection, which applies only when nothing was named - tsc_cmd.rs, lint_cmd.rs, main.rs: thread that runner from the invocation - utils.rs: detection can be asked about a directory, so its test needs no chdir - bun_cmd.rs, rules.rs: strip ANSI on the pm ls text fallback, and stop bun build claiming savings it does not deliver
- utils.rs: MissingTool says whether the npm arm may fetch, instead of every newly routed caller inheriting --no-install by accident - tsc_cmd.rs: tsc fetches as it always did, lint and the rest still do not
Treat pre-existing extensions without ownership state as uncertain, cover project-scope aliases, clean canonical sidecars after symlink removal, and classify protected extension overwrites as breaking. BREAKING CHANGE: non-interactive installs of modified or unrelated Pi/OMP extensions now require --auto-patch to approve overwrites.
Failure detection, so a green run is never reported red: - a failure marker vouches for the block it closes, so a frameless failure such as a timeout keeps the line that says why it failed - bun's marker and run footer are matched with the duration and test count they carry, since a test is free to log a line that starts the same way - deno's FAILURES entries are matched by shape, because a bare " => " also occurs in arrow functions and assertion messages - deno fences output it did not write, and names the fence for when it was printed, so the rule is matched by shape and gates both the failures list and the block engine Diagnostics that were reaching nobody: - deno's type-check errors are kept with the frame under them: they open no block and carry no section, so a type error left the caller with a bare "Type checking failed." - bun's frames are kept, since two failing tests that share a name, and a module error that has no marker at all, cannot otherwise be located Runs whose output rtk must not touch: - watch mode on every filtered entry point goes through unfiltered, since the runners buffer the whole stream until a child that never exits - a deno reporter the user named is left alone, because --reporter=junit and --junit-path write the report to stdout - bun build and deno compile run unfiltered: without an output flag the bundle is stdout, with one the emitted-file summary is the point of the run, and on failure the diagnostics are, none of which an errors-only filter keeps Tool resolution: - a package runner the user named wins over lockfile detection, and over a tool that merely happens to be on PATH - lockfile detection never resolves through bunx, which always fetches a missing tool and cannot be told not to - a tool rtk may fetch resolves through npx, the only runner that can Input and output: - bunx forwards the caller's stdin when it is a pipe, since the filtered path buffers output and an inherited terminal would let a prompt wait invisibly - strip the resolution lines bun actually prints when piped, count the tree levels drawn with a leading pipe, and stop injecting --json into a command the user typed - the lint label records the arguments once, and the shell runners' placeholder tool name stays out of their verbose line Fixtures are real bun 1.2.20 and deno 2.9.6 captures for each case. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A subcommand that runs unfiltered cannot save anything, so classify_command zeroes its estimate rather than letting it inherit the rule's headline. The status and the percentage were separate fields and only the percentage reaches the projection, so marking a subcommand passthrough left the claim standing. The bun and deno percentages are measured against real output rather than against fixtures shaped to the filter: bun install 70, deno lint 40, deno check 50. Each is backed by a captured fixture the module's own test measures. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
feat(bun,deno): add Bun and Deno runtime support
ci(stale): more op per run
…ule range kept GNU diff translates `Only in` and `Binary files … differ` (git translates none of its own lines). Rule 6 read the English spelling only, so on a French machine `diff -ru g1 g2 | rtk diff -` dropped the three translated fact lines as prose and, because a text section still parsed, returned `Some` -- four files became one, silently. Rule 3b now reads GNU diff's per-file `diff <opts> X Y` echo (printed whenever it compares directories) as the mark of a `diff -r` stream. Such a stream carries no prose, so a column-0 line no arm read there is a fact in a language the parser does not speak, and rule 9 returns `None` instead of dropping it. A line dropped before the echo settled the question counts the same way once it does -- `diff -r` may list an `Only in` first. Real fixture `diff_ru_fr_raw.txt` (`LC_ALL=fr_FR.UTF-8`, diffutils 3.10) pins the raw fallback; the English `diff_ru_raw.txt` still parses in full, as do `git log -p` streams whose prologue is prose. The `Submodule <path> <a>..<b>` note now carries the range -- `[file] sub (submodule e139196..b0ac9b1)` -- for parity with the dirty arm's `(submodule, modified content)`.
Groups matches by file, caps at 5/file and 50 total. --json passes through untouched. ~85% savings on a repo-wide test search. Closes rtk-ai#3856
…gitignored matches `rtk find /tmp/nope` exited 0 with no output where `find` prints `find: '/tmp/nope': No such file or directory` and exits 1. The dispatcher read a nonexistent path as RTK name-pattern syntax (`!is_dir()` is true for both), rewrote it to `-name /tmp/nope`, defaulted the search root to `.`, and the existence guard then checked `.` instead of the argument. A token with a path separator can never be a `-name` pattern (`-name` matches basenames), so treat it as a path whether or not it exists; find's own status then surfaces. `run`, `run_compress`, `run_verbatim` and `run_from_args` return the status instead of calling `process::exit`, and the `Find` arm in main.rs yields it. The native walk skips hidden and gitignored entries, so `rtk find . -name secret.txt` printed nothing for a gitignored file and exited 0 — an existence check reads that as "not on disk". The output now ends with `... (N filtered: hidden or gitignored; dirs listed with / were not searched)` plus a tee file listing them, the shape `rtk ls` already uses, and says so even when nothing else matched. The note rides on both sides of the never-worse guard, since the raw listing never holds filtered entries and would otherwise win on size and drop it. The disclosure is derived from the walk that already happened: for each directory the filtered walk visited, its raw children are listed and any child the walk did not visit was hidden or ignored. Skipped files are named when they match; a skipped directory is reported once as `dir/` and never descended. Cost is one read_dir per visited directory: on ext4, a 116-directory checkout with 3000 gitignored files measures 12-14 ms against 14-21 ms before. (Re-walking with the filters off to count matches inside ignored trees took 7.5 s against 350 ms on the same checkout, which is why it is not done.) On a Windows mount under WSL each read_dir is a 9P round-trip, so the disclosure adds roughly 2 ms per visited directory there. Bare names without a separator keep RTK's pattern semantics (`rtk find Cargo.toml`); a pattern starting with `.` still walks hidden entries. Fixes rtk-ai#3851
… root, align the note with ls Review follow-ups on the disclosure added for rtk-ai#3851. - The walk kept every visited path in a set for the whole run, so on a 108k-entry tree RSS went from 7 MB to 18 MB and the extra read_dir pass doubled the time. Disclosure now switches off once 20 000 entries have been visited and the bookkeeping is dropped; below that nothing changes. Measured on /usr: 3.9-4.0 s and 9.6 MB against develop's 3.9-7.0 s and 6.9 MB. - A symlinked search root reported its file type as a symlink, so it never counted as a visited directory and its filtered children went unreported. The root's type is now read through the link. - The note is `... (N filtered)`, the same shape rtk ls emits; the trailing `/` on a reported directory already says it was not searched. - `.git` comes from a named constant instead of an inline literal. - The fixture test disables global gitignore rules so a developer's own excludes cannot change its result; two tests point RTK_TEE_DIR at a tempdir instead of writing under ~/.local/share/rtk. - find_cmd.rs is included through automod, which cargo fmt does not see; the file is now rustfmt-clean when checked directly.
…dens fix(find): report a missing path like find does, and disclose hidden/gitignored matches
The env-prefix stripper treated `sudo` like `env` / `VAR=val` and rewrote `sudo docker ps` into `sudo rtk docker ps`. That breaks at runtime: `rtk` lives in ~/.local/bin, which is not on sudo's secure_path, so the rewritten command fails with "rtk: command not found" under root (reported in rtk-ai#146). And where rtk *is* on secure_path, `sudo rtk` would run the whole rtk binary as root — an unnecessary-privilege footgun. Drop `sudo` from the env-prefix regex so sudo commands pass through untouched. The permission verdict path is unaffected (it never used this regex and already matches sudo commands as-is, e.g. `sudo:*` rules). env / VAR= prefixes and transparent builtins (noglob, command, …) still rewrite normally. Verified: `sudo docker ps` / `sudo -u root docker ps` / `sudo noglob git status` are no longer rewritten; `env FOO=bar docker ps`, `FOO=bar docker ps`, `noglob git status` still are. fmt/clippy clean, full test suite green. Refs rtk-ai#146 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ting names count_find_names and count_find_total treat every unrecognised line as a row of names. Since find began reporting hidden and gitignored matches, its output ends with `... (N filtered)` and a `[see remaining: ...]` pointer, so `rtk find '*' --max 10` counted 19 names (10 + 3 + 6 words) and the --max cap check failed. Skip both lines, as the header, `+N more` and `ext:` lines already are.
…ure-lines fix(benchmark): skip find's disclosure note and tee pointer when counting names
Next Release
…master--components--rtk chore(master): release 0.48.0
refactor(lexer): consolidate raw-command lexing behind shared primitives
fix(ci): unbreak the golangci-lint benchmark row
The "Env Prefix Handling" section of src/discover/README.md still listed `sudo` as a stripped prefix and used it in the chained example; two comments in registry.rs made the same claim. The `sudo head` case in test_head_tail_honour_exclude_commands no longer tested exclusion — it returns None whether or not `head` is excluded, because sudo blocks the rewrite outright. Dropped it; the `RUST_LOG=debug tail` case still covers the env-prefix path. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
fix(rewrite): stop rewriting sudo commands (pass them through)
Prevent Windows checkout conversion from corrupting embedded hook fixtures and make the CRLF regression independent of the source checkout line endings.
feat(omp)!: add Oh My Pi (OMP) support
Review nit: --json is the largest-payload path, so borrow rather than clone result.stdout when skipping the filter.
develop now carries the byte-equality verdict (0cb34ac, 428af61), so this branch no longer argues for it. The alignment fix and the reporting that depends on it remain. compute_diff compared line N to line N, so a single inserted or deleted line reported every line after it as modified. This aligns by Myers' greedy edit script, so one insertion reads as one insertion. The script folds into hunks, one per run of differing lines with no matched line between. Both renderers group by hunk: the classic fallback prints one NcM per region, as diff does, because the classic format cannot express which old line became which new line, and rendering the pairing split a region into an NcM plus a trailing NaM that diff never prints. Inside a hunk the condensed render pairs old and new lines by best similarity rather than by position, so an insertion at the head of a run no longer shifts every pairing after it into a rewrite that did not happen. Op::Keep carries a run length. One entry per matched line made the script scale with the file rather than with the change, 45MB of separators for a million-line pair with 700 rewrites. The aligner's trace is one flat vector with each round's bookkeeping stored inline, so MAX_TRACE_CELLS charges what is allocated: a per-round Vec cost 5.8x the cells being counted on a lopsided pair. The bool develop passes into render_diff cannot express the third case the aligner produces, a comparison that ran and then refused to build a listing because it would cost more than the raw text. FileComparison replaces the bool with Identical, InvisibleDifference and Lines, and DiffResult::unaligned carries the refusal reason. An empty change list is not a synonym for identical, and routing a refusal through the identical branch exits 0 on files that differ. The invisible-difference message keeps develop's wording, so the phrase diff_byte_accuracy_test matches on survives, and then names the measured cause instead of stopping at "no line-content change". It is shown whenever the case arises. A fixed token allowance above raw could not be met: the message's shortest form is ~20 tokens and a one-line pair ~2, so the ceiling sat under the message's floor and dropped it on 90% of one-line pairs, printing the two indistinguishable blobs it exists to replace. The render drops the file-pair header and the blank line, names the files in the frame legend by argument position instead of by path, and prints the counts line only once the listing runs to a screenful. On 120 agent-sized diffs the framing cost more than the change list saved: the body alone is 7% smaller than diff, and each line of chrome erased that.
…dequoted renames `\ No newline at end of file` describes the line directly above it. The parser never renders context lines, so a marker git attached to an unchanged last line landed under the last `-`/`+` line rendered and claimed THAT line lacked a newline -- the one place the output stated something untrue about the diff. The marker is now kept exactly when the line above it was emitted (in-hunk after a `-`/`+`, or on the line right after the budget closed on one), which keeps the witness of a newline-only change and drops a marker that would describe the wrong line. `emitted_at` replaces `closed_at`: the position test is the same for both rules. Real fixture `git_diff_no_newline_raw.txt` carries both shapes. `# HG changeset patch` opens a message region like an mbox `From`, so `hg export`'s column-0 headers and message no longer latch `dropped_prologue` and send the whole changeset raw through its own `diff -r <a> -r <b> <file>` echo. Real fixture `hg_export_raw.txt` (Mercurial 7.0.1) joins the corpus. `rename to`/`copy to` (and their `from`) go through `dequote`, so a `core.quotepath` name has one spelling across a stream. The submodule note keeps the range's direction qualifier: `[file] sub (submodule e139196..b0ac9b1 rewind)`. The body-line replay in `corpus_every_marked_body_line_survives` shares the parser's per-parent presence rule (there is no second formula for combined-diff columns); its doc now says so and names the oracle -- git's header counts -- and it asserts a budget is never still owed at a non-body line or at EOF, so under-consumption fails there too.
Codex and Vibe both return empty rule vectors; keeping them as separate arms multiplies branches that have to stay in sync. Group them, sorted, with one comment covering both. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
feat(hooks): add direct Codex command rewriting
`RtkRule::pipeline_final_safe` was replaced by the `PipelineSafety` enum on develop (rtk-ai#3171). The ast-grep rule still set the old field, so the branch merged cleanly but failed to compile (E0560). `ProducerOnly` keeps the rule's original intent: safe as a pipeline's first stage, never as its final stage, since run() execs with stdin null. Adding a producer-safe rule also requires listing it in `test_pipeline_producer_safe_rule_set`, which pins the exact set. `savings_pct` was left at the 60.0 default; the repo-wide measurement is 84.6%, which is what README and what-rtk-covers.md already claim. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…output Two defects in `filter_ast_grep`, both verified against ast-grep 0.45.3. `ast-grep scan` diagnostics parse only on their locator line, so filtering line by line kept ` ┌─ a.rs:2:13` and dropped the rule id, severity, message and source line — at exit code 0, so it read as success. The whole-output fallback never fired because `order` was not empty. `unparsed_signal()` now passes any shape through untouched when a single non-blank line fails to parse, which is what `search.rs` already does for grep/rg. This also covers `--heading` mode and Windows drive-letter paths, where `[^:]+` cannot match `C:\src\a.rs`. The per-file overflow hint was computed as `entries.len() - max_per_file`, which ignores `max_total` cutting a file short: a file under its own cap lost its remainder with no hint at all, and a file over it under-reported the drop. Both now count against what was actually printed. The hint said "matches" while counting lines. ast-grep prints one line per matched source line and a structural match spans several, so a repo search reported "19 more matches" where five matches remained. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`gh pr checks` spells a pending bucket `pending`, never `*`, and prints five tab-separated fields with the description last. The transition test used a shape gh does not emit; the new case carries the trailing description column and a pending row that resolves across polls. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
feat(ast-grep): add ast-grep filter (rtk ast-grep)
…icate fix(gh): deduplicate watched PR checks
Filtering a failing `gh pr checks` makes the summary the only thing the user sees, so a bucket it does not count disappears. `parse_pr_check_line` dropped every row that was not pass/fail/pending, which turned a cancelled run -- the usual outcome when a new push supersedes CI -- into "Passed: 0, Failed: 0", and left 5 of PR 3982's 15 checks out of the totals. Count the remaining buckets and report them, so the summary always adds up to the checks gh listed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
fix(gh): don't early exit on failure for "pr checks"
Move the weighted-rate regression test off the real tracking database, which it wrote two rows into on every run, and onto an in-memory tracker. Add the long-tailed pair that pins the weighted rate against the mean of percentages, the same assertion through get_summary(), and a zero-input group: without the CASE guard that last one fails on a NULL rate, and no other test covers it. Name the 4th field of GainSummary::by_command for what it now holds, and state the rate formula on the CommandStats warning in percentage order, with the zero guard, so that following it verbatim cannot reintroduce the NULL. Head the column Total% rather than Rate, keeping the shape of the Avg% it replaces and naming the total the figure is now taken over. Drop the hand-written CHANGELOG entry: release-please generates it from the commit subject, and CONTRIBUTING.md asks for that file to be left alone. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
fix(gain): use weighted savings rate in per-command stats
…n memory `get_by_command` moved to the volume-weighted rate while this branch was open (rtk-ai#891), summing every call and guarding the division at group level. The two telemetry queries now use the same shape: the row-level `WHERE input_tokens > 0` is gone, so a call with no input that still printed something counts against its command exactly as it does in the By Command table, and `HAVING SUM(input_tokens) > 0` keeps commands that never had any input out of both, as before. `low_savings_commands` lists net-regressing commands: `sav > 0.0` became `sav <> 0.0`, so only exact 0% stays with `passthrough_top`. A command that emits more than it saves is the filter to fix first, and with the weighted rate the old bound also dropped commands the unweighted query used to list. The two regression tests opened the on-disk tracker, so every `cargo test` wrote ten synthetic rows (one at 100k input tokens) into whichever history database `Tracker::new()` resolves to, on a developer machine the real one. They now use `Tracker::new_in_memory()` like the rest of the module, and `test_avg_savings_per_command_inner_weighted` only asserted a `0..=100` range that the unweighted query also satisfied. Both tests now assert the exact weighted figures, check each listed rate against `get_summary()`, and fail when either query goes back to `AVG`, to the row filter, or to the positive bound. Tests are numbered 18 and 19 after the ones develop already carries as 14-17. The `low_savings_commands` doc no longer points at a PR, and both telemetry field tables describe the new semantics. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…egations fix(tracking): weighted savings rate in low_savings_commands and avg_savings_per_command
The pipeline diagram, the discover walkthrough and the `rtk read` option table still described the `--max-lines` mapping this branch replaced, and the repo's own suggest hook still proposed `--max-lines N` for `head -N` — the mapping this branch exists to correct. Drop the `head -` disjunct in the rewrite guard: every string with that prefix also has the `head ` prefix that sits next to it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`rewritten_head_spellings_match_native_on_non_utf8_files` isolates `CLAUDE_CONFIG_DIR` but reads the ambient `XDG_CONFIG_HOME`, so a config carrying `exclude_commands = ["head", "tail"]` suppresses the rewrite and the exit-code-3 assertion fails. Clean runners have no such config, so the test passes in CI and fails only for the developers most likely to have opted out of these very rewrites. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…dows fix(read): make head/tail rewrites faithful to the native commands
Pins both directions: the commands that report hook state themselves stay quiet, and the filtered commands still warn. Also pins the reason the suppression is worth having — `maybe_warn` touches the once-a-day marker, so warning on a meta command would leave the next filtered command silent. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…-during-init fix: suppress hook warning during rtk init and verify
Make --opencode additive so global initialization installs the Claude hook and OpenCode plugin together. Update user-facing guidance and add CLI regression coverage. Fixes rtk-ai#4046 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: TechWizard9999 <119874392+TechWizard9999@users.noreply.github.com>
- Modify record_tee_elision() to create the recall store if needed - Modify record_tee_recall_with() to create the recall store if needed - Update misleading message in gain --recalls to mention tee mode This allows tee-only users to see their recall stats in 'rtk gain --recalls'. The stats table is lightweight and separate from the full recall store. Fixes rtk-ai#4219.
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
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.
Summary
This PR fixes issue rtk-ai#4219 where tee-only users couldn't see their recall stats in
rtk gain --recalls.Changes
src/core/retriever.rs: Modified
record_tee_elision()andrecord_tee_recall_with()to create the recall store (SQLite database) if needed when recording stats. Previously, these functions usedopen_existing()which only opens an existing database, so tee-only users never had their stats recorded.src/analytics/gain.rs: Updated the misleading message in
show_recall_stats()to mention that stats work in both sqlite and tee modes.Root Cause
The issue was that in tee mode, the recall store was intentionally never created (to "leave no sqlite artifact behind"). However, the
recall_statstable schema supports "tee" mode, andrtk gain --recallsdisplays tee stats - but tee-only users could never populate them because the database was never created.The fix allows the stats table to be created on-demand when the first stat needs to be recorded. The stats table is lightweight (just counters) and separate from the full recall store that stores command outputs.
Testing
rtk gain --recalls