feat(#155): Bash matcher opt-in surface, telemetry, and doctor integration (PR 2/2) - #217
Merged
robotrocketscience merged 5 commits intoApr 28, 2026
Conversation
Owner
Author
|
Kulili review (Sonnet output) — clean. Green from my side. What I checked
Non-blocking observations
Merge order reminderPR #217 is based on PR #213 ( Otherwise this is ready. |
…155 AC3) Appends one JSONL record per Bash-matcher fire to <git-common-dir>/aelfrice/telemetry/search_tool_hook.jsonl. Schema: timestamp, session_id, command, query, latency_ms, injected_l1, injected_l0. Ring cap: 1000 entries (oldest evicted, atomic tempfile+os.replace). Write is fail-soft: any OS error prints one stderr line and the hook continues. Adds public read_telemetry(path) -> list[dict] helper for the doctor surface; raises ValueError on corrupt JSON, returns [] when file is missing. Latency clock starts at Bash-branch entry (time.perf_counter).
13 deterministic tests covering: record write, accumulation, ring-cap eviction (1000-entry ceiling), fail-soft on unwritable path, missing file returns [], corrupt JSON raises ValueError, blank-line skipping, non-dict JSON skipping, timestamp presence, and an end-to-end invocation through main() with a monkeypatched :memory: DB.
#155 AC7) Adds install_search_tool_bash_hook / uninstall_search_tool_bash_hook to setup.py, wired as a separate PreToolUse:Bash entry sharing the same aelf-search-tool-hook entry point. The Bash matcher entry coexists with the Grep|Glob entry (same command, distinct matcher field). CLI: aelf setup --search-tool-bash installs idempotently; --no-search-tool-bash removes it. aelf unsetup --search-tool-bash also removes it. All paths are independent of --search-tool (v1.2.x flag). 15 deterministic tests cover install, idempotency, removal, round-trip, coexistence with Grep|Glob, and the full CLI surface.
…AC8) Adds diagnose_search_tool_telemetry(path) to doctor.py: reads the Bash matcher JSONL ring buffer and computes fire_count, p50/p95 latency, and injection-noise rate (fraction of fires with zero L0+L1 hits). DoctorReport gains search_tool_telemetry, search_tool_telemetry_path, and search_tool_telemetry_corrupt fields. diagnose() accepts an explicit search_tool_telemetry_path; falls back to deriving it via git-common-dir. format_report() renders the section after the existing hooks output; renders even when no settings.json is found (section is independent). Sentinel: missing/empty file → "no fires recorded". Corrupt file (bad JSON) → CORRUPT flag in report and text. 16 deterministic tests.
robotrocketscience
force-pushed
the
feat/issue-155-search-tool-bash-setup-telemetry
branch
from
April 28, 2026 21:06
7c57e7b to
b2073af
Compare
robotrocketscience
deleted the
feat/issue-155-search-tool-bash-setup-telemetry
branch
April 28, 2026 21:07
This was referenced Apr 28, 2026
robotrocketscience
added a commit
that referenced
this pull request
Apr 28, 2026
…) (#252) Closes #218. ## Summary Adds an observability surface to the `UserPromptSubmit` hook so dedup / relevance regressions become visible without a human eyeballing context. Mirrors the `search_tool_hook` telemetry pattern shipped in #217. - **AC1-3, AC5 — Telemetry ring buffer.** Per-fire JSONL append at `<git-common-dir>/aelfrice/telemetry/user_prompt_submit.jsonl`. Schema: `{timestamp, query, n_returned, n_unique_content_hashes, n_l0, n_l1, total_chars}`. `query` capped at 500 chars. Ring cap 1000; atomic tempfile + `os.replace`. Fail-soft on any write error (single stderr line, hook continues). Public `read_user_prompt_submit_telemetry(path)` for the doctor surface. - **AC4 — `aelf doctor` surface.** New `diagnose_user_prompt_submit_telemetry()` reports rolling p50/p95 of `total_chars` and median dedup-collapse-rate (`n_returned / n_unique_content_hashes`). Missing/empty → \"no fires recorded\". Corrupt JSONL → CORRUPT sentinel. - **AC6 — `collapse_duplicate_hashes` flag.** New `[user_prompt_submit_hook] collapse_duplicate_hashes` key in `.aelfrice.toml`, default OFF. When ON, hits are deduped by content hash (first-occurrence wins) before formatting. Telemetry records the *pre-collapse* `n_returned` so the dedup-rate metric stays meaningful; `total_chars` reflects what was actually injected. Telemetry only fires when the hook produces an `<aelfrice-memory>` block (empty hits → no record). This is the observability + workaround track. Algorithmic dedup remains #197; topic-relevance work remains #154. ## Test plan - [x] `tests/test_user_prompt_submit_telemetry.py` — 14 tests: write, accumulation, ring-cap eviction, fail-soft on unwritable parent, missing file, corrupt JSON, blank-line skip, end-to-end via `user_prompt_submit()` - [x] `tests/test_user_prompt_submit_collapse.py` — 16 tests: config load (default-off, on, malformed TOML, wrong type, missing file, missing section), dedup ordering, telemetry pre/post-collapse semantics - [x] `tests/test_aelf_doctor_user_prompt_submit_telemetry.py` — 17 tests: missing → sentinel, populated → p50/p95 + collapse-rate, corrupt → CORRUPT, formatter rendering - [x] Full suite: 1692 passed, 8 skipped (pre-existing platform skips) --------- Co-authored-by: rrs <jonsobol@gmail.com>
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
1 participant
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
Completes the v1.5.0 #155 Bash extension split. PR 1 (parsers + allowlist) is at #213.
AC3 — Telemetry ring buffer.
hook_search_tool.pyappends one JSONL record per Bash-matcher fire to<git-common-dir>/aelfrice/telemetry/search_tool_hook.jsonl(schema:timestamp,session_id,command,query,latency_ms,injected_l1,injected_l0). Ring cap 1000 entries; atomic tempfile+os.replace. Fail-soft on any write error. Publicread_telemetry(path)for the doctor surface.AC7 —
aelf setup --search-tool-bash/--no-search-tool-bash. Installs a separatePreToolUse:Bashhook entry (sameaelf-search-tool-hookentry point;matcher="Bash"field distinguishes it from the v1.2.xGrep|Globentry). Both flags are idempotent and independent of--search-tool.aelf unsetup --search-tool-bashalso works.AC8 —
aelf doctortelemetry section. Newdiagnose_search_tool_telemetry()computes rolling p50/p95 latency and injection-noise rate.format_report()renders asearch_tool_hook telemetry:block. Missing/empty → "no fires recorded". Corrupt file → CORRUPT sentinel.Test plan
tests/test_search_tool_hook_bash_telemetry.py— 13 tests: write, accumulation, ring-cap eviction, fail-soft, missing file, corrupt JSON, blank-line skip, end-to-end viamain()tests/test_aelf_setup_search_tool_bash.py— 15 tests: low-level install/uninstall idempotency, coexistence with Grep|Glob, CLI--search-tool-bash/--no-search-tool-bash,aelf unsetup --search-tool-bashtests/test_aelf_doctor_search_tool_telemetry.py— 16 tests: percentile computation, noise rate, diagnose() integration, format_report() renderingOpen questions (documented, not blocking)
bash_fire_cap_per_turnin.aelfrice.toml) — deferred to v1.5.x per spec.Refs #155 (PR 2/2; PR 1 = #213)