Skip to content

docs: search-tool hook Bash matcher extension spec (#155) - #209

Merged
robotrocketscience merged 1 commit into
mainfrom
docs/search-tool-bash-spec-v1.5
Apr 28, 2026
Merged

docs: search-tool hook Bash matcher extension spec (#155)#209
robotrocketscience merged 1 commit into
mainfrom
docs/search-tool-bash-spec-v1.5

Conversation

@robotrocketscience

Copy link
Copy Markdown
Owner

Refs #155.

Summary

Adds a § Bash extension section to docs/search_tool_hook.md resolving the v1.5.0 carryover question on extending the matcher beyond Grep|Glob.

Decision: narrow allowlist of search-shaped Bash commands (grep / rg / ack / find / fd) behind a separate opt-in flag, default-OFF at v1.5.0. Default-on flip gated on telemetry: latency p95 ≤ 200 ms AND injection-noise rate ≤ 30 % over ≥ 200 representative fires.

ls, cd, cat, pipes, command substitution → silent-skip. Per-command micro-parsers, no fall-through to "match all Bash". Per-turn fire cap of 3 to prevent pipeline storms. Halved token budget (300 / 5) vs. the v1.2.x path (600 / 10). Telemetry surface at .git/aelfrice/telemetry/search_tool_hook.jsonl + aelf doctor section. Independent opt-in flag (--search-tool-bash).

8 acceptance criteria, per-criterion test plan, out-of-scope list, 3 deferred-to-implementation questions.

Test plan

  • Doc-only change — no source edits, no tests
  • Reviewer reads § Bash extension end-to-end and confirms the allowlist + parser contract
  • Reviewer accepts the telemetry-gate decision (default-off until p95 + noise budget hold over ≥ 200 fires)

Adds a § Bash extension section to docs/search_tool_hook.md (the
v1.2.x search-tool hook spec) covering the v1.5.0 carryover
question of whether to extend the matcher beyond Grep|Glob.

Decision: extend to a narrow allowlist of search-shaped Bash
commands (grep/rg/ack, find, fd) behind a separate opt-in flag,
default-off at v1.5.0, default-on flip gated on telemetry showing
latency p95 \u2264 200 ms AND injection-noise rate \u2264 30 % over
\u2265 200 representative fires.

Spec covers:

- Allowlist (per-command query field mapping; ls / cd / cat
  excluded as no-search-intent)
- Per-command micro-parsers; pipelines and command substitution
  silent-skip rather than guess
- Per-turn fire cap of 3 to prevent pipeline storms
- Halved token budget vs. the v1.2.x Grep|Glob path (300 / 5)
- Telemetry surface at .git/aelfrice/telemetry/search_tool_hook.jsonl
  + aelf doctor section
- Independent opt-in surface (--search-tool-bash, separate from
  the v1.2.x --search-tool flag)
- 8 acceptance criteria + per-criterion test plan
- Out-of-scope list and 3 deferred-to-implementation open questions

This satisfies the #155 "spec must merge before impl" gate. Impl
work tracks separately.
@robotrocketscience
robotrocketscience merged commit 27dd166 into main Apr 28, 2026
8 checks passed
@robotrocketscience
robotrocketscience deleted the docs/search-tool-bash-spec-v1.5 branch April 28, 2026 20:41
robotrocketscience added a commit that referenced this pull request Apr 28, 2026
Refs #155. Builds on the spec in #209 (must merge first; this PR's base
is `docs/search-tool-bash-spec-v1.5`).

## Summary

PR 1 of 2 for the v1.5.0 §Bash extension. Lands the per-command parsers,
allowlist, fire-cap, and source-attribute output shape.

PR 2 (follow-up) will land the opt-in surface (`aelf setup
--search-tool-bash`), telemetry to
`.git/aelfrice/telemetry/search_tool_hook.jsonl`, and `aelf doctor`
integration — that's where ACs 3, 7, 8 close. Default-OFF until PR 2
lands the opt-in flag, so v1.5 cut can ship this without flipping
behaviour.

## What this PR does

- Allowlist:
`grep`/`egrep`/`fgrep`/`rg`/`ripgrep`/`ack`/`find`/`fd`/`fdfind`.
Per-command flag-with-arg tables. Strict `find` rule (only
`-name`/`-iname` produce a query; `find . -type f` silent-skips).
- Pipeline / command-substitution / backtick / process-substitution /
redirection / shell-control tokens abort the parser. No fall-through to
"match all Bash."
- Env-assignment prefixes (`RUST_LOG=trace rg ...`), `nohup` / `time` /
`command` / `exec` wrappers, and `/usr/bin/grep` basenames are all
recognised.
- Per-turn fire cap of 3, keyed by `session_id` in a per-process dict.
Distinct sessions each get the full cap. Cap applies only to the Bash
matcher; Grep|Glob is uncapped.
- Halved token budget (300 / 5) for the Bash matcher vs. the v1.2.x
Grep|Glob path (600 / 10).
- Output shape: `<aelfrice-search query="..." source="bash:rg" cmd="rg
searchterm src/">...</aelfrice-search>`. Grep|Glob output is
byte-identical to v1.2.x.

## Acceptance criteria

| AC | Status | Coverage |
|---|---|---|
| 1 — allowlist fires; unknown silent-skip | done |
`test_allowlist_fires_*`, `test_unknown_command_silent_skip`,
`test_random_garbage_never_fires` |
| 2 — per-command exact extraction | done | 10 parser unit tests
covering documented shapes + abort cases |
| 3 — telemetry-gated default-on flip | **deferred to PR 2** | telemetry
surface lands in PR 2 |
| 4 — no fall-through to arbitrary Bash | done |
`test_no_fall_through_to_arbitrary_bash`,
`test_unknown_tool_name_silent_skip` |
| 5 — per-turn fire cap | done | `test_per_turn_fire_cap_holds`,
`test_fire_cap_independent_per_session` |
| 6 — `source=` / `cmd=` attribute | done |
`test_bash_block_carries_source_attribute`,
`test_grep_block_unchanged_no_source_attribute` |
| 7 — `aelf setup --search-tool-bash` | **deferred to PR 2** | |
| 8 — `aelf doctor` telemetry | **deferred to PR 2** | |

## Test plan

- [x] `uv run --extra dev pytest tests/test_search_tool_hook_bash.py` —
26 pass
- [x] `uv run --extra dev pytest tests/` — 1489 pass / 4 skip (no
regressions)
- [ ] CI staging-gate green
- [ ] Reviewer accepts the PR-1/PR-2 split
- [ ] Reviewer accepts the naive-whitespace-split fail-soft contract
(quoted args with embedded whitespace silent-skip rather than guess)
robotrocketscience added a commit that referenced this pull request Apr 28, 2026
…gs + telemetry (#232) (#234)

Closes #232.

Implements the v1.5 retrieval composition plumbing split off from #154
(#154 retargeted as the v1.7 default-on-flip tracker).

## What ships

Three atomic commits:

1. **`refactor(retrieval): rename bm25f_enabled -> use_bm25f_anchors per
#154 flag policy`**
Standardises the v1.5 flag-name surface on `use_<lane>_<modifier>`.
Renames kwarg / TOML key / helper across `retrieve()`,
`retrieve_with_tiers()`, `retrieve_v2()`, `RetrievalCache.retrieve`.
`AELFRICE_BM25F` env var unchanged. #148 just shipped so no
released-flag breakage.

2. **`feat(retrieval): placeholder flags + per-lane telemetry`**
Four placeholder flags (default-OFF, no-op): `use_signed_laplacian`,
`use_heat_kernel`, `use_posterior_ranking`, `use_hrr_structural`.
Setting any true in `.aelfrice.toml` emits one stderr warning per
process per flag, naming the flag and the tracker issue. Adds
`LaneTelemetry` dataclass + `last_lane_telemetry()`; populated in
`retrieve()` / `retrieve_with_tiers()` with one assignment each.

3. **`test: composition-tracker placeholder flags + LaneTelemetry`**
8 deterministic tests: warner once-per-process idempotency, empty-TOML
quiet, explicit-False quiet, `PLACEHOLDER_FLAGS` regression guard,
`LaneTelemetry` populates for FTS5 default + `use_bm25f_anchors=True`,
posterior_weight round-trip.

## Acceptance (per #232)

- AC1: rename complete across all four surfaces ✅
- AC2: four placeholder flags resolve through env > kwarg > TOML >
default ✅
- AC3: stderr warning idempotent per flag per process ✅
- AC4: `LaneTelemetry` populates for both L1 lanes + posterior
round-trip ✅
- AC5: byte-identical regression tests preserved (full suite was green
at HEAD pre-rebase: 1471 pass / 4 skip; this PR rebases onto current
`main` clean and CI staging-gate revalidates).

## Out of scope

- No default flips. No new retrieval algorithms. No benchmark gate.
Tracked under #154 for the v1.7 wave.

## Note on rebase

Branch was 6 commits behind `main` after #211 / #221 / #231 / #202 /
#210 / #209 landed. Rebased onto `github/main` with no conflicts.
Force-pushed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant