Reflective sprint: six load-bearing improvements - #12
Merged
Conversation
A small plain-text file at the root of (or anywhere above the cwd inside) your project. Its contents become a "Project context" system message that prefixes every `,` and `?` invocation in that tree. $ cat .shellllmrc This project uses pnpm (not npm). We target Python 3.11+. Prefer ripgrep over grep. $ , find files modified today → proposes `rg --files --max-age 1d ...` instead of `find` Discovery walks up from cwd, stops at $HOME (we never read system- wide rc files), innermost file wins — same precedence as .editorconfig / .gitignore. Read fresh every call so edits take immediate effect; capped at 4KB with tail truncation so a runaway file can't blow out the context budget. Wired through both `,` (comma.py _build_messages) and `?` (ask.py run_agent) so any English-prompt or repair call picks the context up. Doesn't touch `???` recall — that's archive search, not new conversation. 9 new tests cover discovery (cwd, walk-up, innermost-wins, stops at HOME), rendering (system message format, blank-file handling), and the size cap. Today only the system message is documented for users to put project conventions in. The file format stays plain text so we can add structured keys later without breaking anything.
The diagnose-then-suggest design only works as well as the model's category assignment. On the fast tier I measured the kubectl auth-failure case getting misclassified as 'Logic:' — the model forced a category because the prompt offered no out, and the suggestions degraded accordingly. Two tightenings: 1. **Five worked examples** baked into the system prompt, one per category, showing the exact shape (`previous command:` / `output:` / `→ Category: ...` / `→ first command: ...`). Smaller models match the pattern reliably; even the fast tier (3B-active MoE) now classifies the kubectl case correctly. 2. **'Uncertain:'** as a fourth valid category. When the model genuinely can't tell which of Typo/Environment/Logic applies, it says so — and `_pick` is invoked automatically instead of dropping a potentially-wrong fix on the prompt. Better honest uncertainty than confident wrongness. Live verification (post-change, both tiers): case fast balanced ----------- ---------------- ---------------- du --max-deth Typo ✓ Typo ✓ git log no-repo Environment ✓ Environment ✓ kubectl auth Uncertain ✓ Uncertain ✓ The Uncertain branch composes with --pick: passing --pick still shows the picker; emitting 'Uncertain:' also shows the picker. The two paths converge on the same fall-through code. 275 tests pass, including a new one that mocks an Uncertain diagnosis and asserts the picker fires even without --pick.
The archive grows forever today. No TTL, no cap, no compactor — power users would end up with a 500MB archive.db in six months and recall slows down silently. Three new commands plus a status hint make the archive operable: ??? --prune --older-than 90d drop sessions older than 90 days ??? --prune --keep 1000 keep only the 1000 most recent ??? --prune --older-than 30d --keep 500 both (cuts both ways) ??? --vacuum reclaim disk space after a prune ??? --status now also shows MB on disk Age spec accepts s / m / h / d / w suffixes (`30d`, `12h`, `2w`). Garbage rejected at parse time with a clear error. The Archive class gains three methods (`prune_older_than`, `prune_to_keep_newest`, `vacuum`, plus `db_size_bytes`). The FTS5 search index is kept consistent automatically — the schema's DELETE trigger handles it, no manual rebuild needed. `?? --prune` and `--vacuum` are mutually exclusive with the recall-filter flags (`--ask` / `--comma`) the same way the other global modes are — pruning is global, filtering recall is per-cmd. 29 new tests (archive-level + CLI-level) cover happy paths, edge cases (zero kept, no rows to prune), and the parse rejections.
\`?? --doctor\` runs every check a new user is likely to hit during
setup and prints them in one screen. Exit code is 0 if everything
required is fine, 1 if anything required is missing.
What it checks:
- platform basics (uname, shell)
- required binaries (shellllm-{comma,ask,recall}, llama-server, fzf)
- optional binaries (huggingface-cli — informational, no fail)
- zsh layer sourced
- the three SHELLLM_* env vars that bite people most often
(SHELLLM_SHELL_CONTEXT, SHELLLM_BASE_URL, SHELLLM_AUTOSTART)
- every chat tier's running state + the loaded model name
- embed server (optional, marked as info)
- archive + memory counts and on-disk size
- which tier models are already in the HF cache
Output is plain ASCII (no Unicode glyphs, no colour) so it
round-trips through GitHub issues, Slack pastes, and email
verbatim — copy-paste it into a bug report and the maintainer has
everything they need in one block.
Read-only by design: \`--doctor\` never starts/stops anything, never
touches the archive, never modifies anything in the user's
environment. Safe to run as the first thing in any new shell.
Wired into the \`??\` dispatcher and listed in \`?? --help\`.
The fix-mode no-picker case (the bare ,, that 90% of users will hit
90% of the time) used to wait for the full JSON to arrive before
doing anything. On the balanced tier that's 5-10s of "thinking…"
for what amounts to a typo correction the model knew in the first
half second.
Now we stream the JSON. As soon as the first complete `{command,note}`
object is parseable AND a non-Uncertain diagnosis has streamed, the
hot path bails out of the iteration and prints the command. End-to-end
latency drops to ~1.3s on the fast tier in live measurement.
What changed:
- client.chat_stream now accepts response_format=json_schema (it
previously only handled the tool-call streaming case). The
server-side stream is unchanged.
- comma._extract_complete_command_items uses json.JSONDecoder.raw_decode
to chip complete {command,note} objects off the streaming buffer one
at a time. Robust against partial strings with escapes.
- comma._try_extract_diagnosis recovers the diagnosis field as soon as
its closing quote has streamed. Earlier "append closing brace" tricks
could surface truncated diagnoses (`Typ` instead of `Typo: ...`); the
raw_decode approach won't fire until the JSON string value has
legitimately terminated.
- comma._stream_command_items wraps both above: yields each command
object as it forms, surfaces the diagnosis via a callback, and lets
the caller break out of iteration early when it has what it needs.
- comma.main's hot path: spinner shows `thinking… (N so far)` updates,
fix-mode-no-pick-no-Uncertain breaks after the first item + diagnosis,
the diagnosis is captured during streaming and surfaced to stderr
*after* the spinner exits (writing to stderr while Rich holds the
cursor was producing mangled output).
The picker path (plain `,`, `, --ctx`, `,, --pick`, and Uncertain
diagnoses) still accumulates the full list before showing fzf —
streaming wins there are smaller and would need a more complex fzf
reload protocol.
Test fixture updates: `fake_model` now patches both chat and
chat_stream. A `_patch_chat_both` helper lets local fakes do the same
without copying boilerplate. All 302 tests pass; no behavior change
for any flow we previously tested.
ruff catches style and basic correctness; mypy catches the kind of
bug that's currently caught only by noticing weird behaviour during
demo renders. With the new streaming hot path and the diagnose-then-
suggest schema work, that's exactly the kind of mistake that gets
through ruff and bites at runtime.
Two pre-existing errors surfaced:
- comma.main: `chosen = _pick(...)` returned `str | None` but was
later used as `str`. Now explicitly typed and narrowed.
- ask.run_agent: two heterogeneous-dict literals (one with tool_calls,
one without) shared a variable name and produced no-redef +
incompatible-dict-item errors. Renamed the second to
`assistant_with_tools` and gave both `dict[str, Any]` annotations.
mypy config in pyproject.toml is intentionally moderate, not strict:
- check_untyped_defs / no_implicit_optional / strict_optional /
warn_unused_ignores / warn_redundant_casts / warn_unreachable
- tests/* is excluded — the monkey-patched fakes there are not the
place to fight types.
CI gets a new step between `ruff format --check` and `pytest`:
- name: Type-check with mypy
run: mypy
Local dev: `pip install -e ".[dev]"` now pulls mypy too.
302 tests pass, ruff clean, ruff format clean, mypy clean.
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.
Six commits, each independently reviewable, addressing the highest-leverage items from the recent reflective review.
What landed
1. `.shellllmrc` — per-project context (commit 1)
A plain-text file at any directory above `cwd` (innermost wins, walk stops at `$HOME`). Its contents become a "Project context" system message that prefixes every `,` and `?` invocation in that tree. Like `.editorconfig` for the LLM. Read fresh every call so edits take immediate effect; 4KB cap with tail truncation.
2. Diagnose-then-suggest reliability (commit 2)
Five worked examples baked into the `,, ` system prompt — one per category — so smaller tiers (fast MoE) classify as well as balanced. Adds a fourth category, `Uncertain:`, that the model can emit when it genuinely can't tell which fix is right; the CLI sees that and falls through to the picker instead of dropping a confidently-wrong fix.
Live verification, both tiers, four cases:
3. Archive lifecycle (commit 3)
The archive grew unbounded. New commands:
```
??? --prune --older-than 90d # drop sessions older than 90 days
??? --prune --keep 1000 # keep only the 1000 most recent
??? --vacuum # reclaim disk space after a prune
??? --status # now also shows MB on disk
```
Age spec accepts `s/m/h/d/w` suffixes; garbage rejected with a clear error. Composes (both flags in one call cuts both ways). 29 new tests.
4. `?? --doctor` health check (commit 4)
One command checks: platform, required & optional binaries on PATH, zsh layer sourced, environment variables that bite people, every chat tier with loaded model name, embed server, archive size + counts, and which tier models are in the HF cache. Plain ASCII output — round-trips through GitHub issues, Slack pastes, and email verbatim. Read-only by design.
5. Streaming for `,` and `,,` (commit 5)
The fix-mode no-picker case (90% of `,,` usage) used to wait for the full JSON. Now we stream:
```
[before] thinking… ~5-10s → print fix
[after] thinking… (1 so far) ~1.3s → diagnosis line + print fix
```
Measured live: 1.27s end-to-end on the fast tier for `du --max-deth` → corrected fix on prompt + diagnosis on stderr. Two robustness wins along the way: `_extract_complete_command_items` uses `raw_decode` to chip complete `{command,note}` objects off the stream (handles partial strings with escapes); `_try_extract_diagnosis` waits for the closing quote of the string value (avoids surfacing a truncated "Typ" when the full "Typo: ..." hasn't arrived yet).
6. mypy type checking in CI (commit 6)
Added between `ruff format --check` and `pytest`. Moderate config (`check_untyped_defs`, `warn_unreachable`, `strict_optional`), not strict. Two pre-existing type errors fixed: a `_pick` result that wasn't narrowed, and a name reuse for `assistant_msg` with incompatible dict shapes.
Test plan