polish: context economy, response quality, and session lifecycle pass 1 - #477
Closed
anandh8x wants to merge 1 commit into
Closed
polish: context economy, response quality, and session lifecycle pass 1#477anandh8x wants to merge 1 commit into
anandh8x wants to merge 1 commit into
Conversation
Addresses post-launch user feedback comparing Zero against opencode on the same model (Minimax M3): larger system prompt, slower despite cache hits, skills not detected first-go, verbose responses, context filling in ~5 prompts, and missing /new. Targets the high-impact, low-risk items; defers the ones needing a maintainer decision (safety-policy rewrite, core-tool deferral activation semantics). Context economy - Compaction: preserveLast 8->6, triggerRatio 0.8->0.7 (both copies in sync). - System prompt: kill the "ELABORATE summary" directive; scale summary to the work. Comment discipline moved into the core prompt for all model families (was Anthropic-only); Minimax/DeepSeek/Qwen/etc. now get it. - Token-budget regression test ratchets the fixed per-turn overhead (base prompt + eager tool schemas) using the runtime's own estimator, so silent prompt/schema growth fails CI. Response quality - defaultResponseStyle balanced->concise. Reversible via /style. Tool discovery - <available_skills> block in the system prompt lists installed skills (name + one-line description) so the model loads the right one on the first call instead of guessing and reading the failure. Budget-capped at 640 bytes with an "...and N more" fallback; renders nothing when no skills are installed (byte-identical to prior prompt). Session lifecycle - /new starts a fresh session in place; previous session stays on disk and is named in the confirmation for /resume. Guards against mid-run reset. - /clear now notes that context persists and points to /new. Performance & caching - partitionTools no longer alpha-sorts the whole tools array every turn, which inserted a loaded deferred tool mid-array and shifted the cached prefix on every load. It now builds append-only regions [eager sorted] + [tool_search] + [loaded deferred sorted] so a mid-session load grows the tail instead of busting the eager prefix. Regression test asserts the eager block precedes any loaded tool. - /context now shows cache hit rate (flags low rate over >5 turns as a possible unstable prefix) and rolling average turn latency with time-to-first-token. Both reset on /new. Measured baselines (auto-mode, no workspace): base prompt 3162 tokens, eager tool schemas 2433 across 12 tools. Budget ceilings set ~10% above. Deliberately deferred (need a decision): inverted safety-policy rewrite (sandbox doesn't catch ~half the B-tier rules), core-tool/web_fetch deferral (activation-semantics blocker), aggressive prompt slim.
WalkthroughThis PR lowers compaction thresholds, stabilizes tool-partition cache ordering, simplifies model-specific prompt addenda, and adds a Skills system-prompt feature wired from CLI plugin activation. The TUI gains a /new command with session-reset logic, per-turn latency/TTFT tracking, cache-efficiency display, and a default style change to "concise." ChangesAgent Prompt, Tools, and Compaction
Estimated code review effort: 3 (Moderate) | ~30 minutes TUI Session Lifecycle, Latency, and Cache Reporting
Estimated code review effort: 3 (Moderate) | ~35 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant model
participant Transcript
participant usageTracker
User->>model: submit /new
model->>model: check pending/compactInFlight
alt run in progress
model-->>Transcript: append cancel-first warning
else idle
model->>model: startNewSession()
model->>usageTracker: reset usage state
model->>Transcript: clear and append welcome+note
end
sequenceDiagram
participant Caller
participant partitionTools
participant ToolRegistry
Caller->>partitionTools: request active tool definitions
partitionTools->>ToolRegistry: list eager tools (alpha-sorted)
partitionTools->>partitionTools: place tool_search after eager block
partitionTools->>ToolRegistry: append loaded deferred tools (alpha-sorted)
partitionTools-->>Caller: definitions with stable prefix
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/tui/model.go (1)
4232-4244: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
OnTextTTFT stamp missing whitespace guard.
OnText'sfirstTokenAtstamp fires on ANY delta, but the symmetricOnReasoninghandler (Lines 4332-4334) only stamps on a non-whitespace delta. A leading whitespace-only text chunk will record an artificially early time-to-first-token, understating the metric shown in/context. This also contradicts the documented behavior ("set the first time a non-whitespace text delta arrives").🐛 Proposed fix
onText := options.OnText options.OnText = func(delta string) { - if firstTokenAt.IsZero() { + if firstTokenAt.IsZero() && strings.TrimSpace(delta) != "" { firstTokenAt = m.now() }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/tui/model.go` around lines 4232 - 4244, The `OnText` handler in `options.OnText` is stamping `firstTokenAt` for any delta, which can record an early TTFT on whitespace-only chunks. Update the `OnText` wrapper to mirror the `OnReasoning` behavior by only setting `firstTokenAt` when the incoming `delta` contains non-whitespace text, while keeping the existing `flushReasoning`, `m.sendAgentText`, and `onText` callback flow intact.
🧹 Nitpick comments (1)
internal/agent/system_prompt.go (1)
183-216: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueEdge case: all-blank skill names still emit an empty block.
If every
Skillsentry has a blankName(all skipped by thename == ""continue), the function still writes the<available_skills>header/intro and</available_skills>closer with zero entries and no overflow line — a misleading empty section telling the model to call a skill tool with nothing to call. Currently unreachable viapluginActivation.skillInfos(which pre-filters blank names), but this function is public within the package and could be called with unfiltered input in future.🛡️ Proposed defensive guard
if omitted > 0 { b.WriteString("- …and " + strconv.Itoa(omitted) + " more (call skill with a name; an unknown name lists them all)\n") } + if listed == 0 { + return "" + } b.WriteString("</available_skills>") return b.String()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/agent/system_prompt.go` around lines 183 - 216, The skillsContext function can emit an empty <available_skills> block when every Options.Skills entry has a blank Name, because the loop skips all entries but still writes the header and footer. Add a defensive guard in skillsContext after filtering/trimming names so it returns an empty string when no valid skill names are present, ensuring the block is only rendered when at least one skill like info.Name is actually included.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/agent/system_prompt.go`:
- Around line 191-216: The skill list builder in system_prompt.go is still
allowing internal newlines from info.Description to flow into a single skill
entry, which can break the <available_skills> formatting. Update the logic
around buildAvailableSkills/available skills rendering so each description is
normalized to a single line before truncateForSkillLine is applied, collapsing
embedded whitespace/newlines in addition to the existing TrimSpace. Keep the
existing name/description handling and budget counting in place, but ensure each
emitted "- name: desc" line stays on one line.
In `@internal/tui/model.go`:
- Around line 1869-1878: The TTFT label in the session metrics is using the
wrong sample count, so the average first-token timing can report an inflated
turn count. Update the `avgTurnLatencyText` logic in `internal/tui/model.go` to
use `turnTTFTCount` for the TTFT average display (or present TTFT and latency
counts separately if both are shown), while keeping `turnLatencyCount` only for
wall-time latency.
In `@internal/tui/new_session_test.go`:
- Around line 12-33: `startNewSession()` is resetting additional state beyond
session metadata and transcript, but `TestStartNewSessionResetsState` only
verifies the visible session fields. Update this test (or the latency-related
test that exercises the same flow) to assert that the `/new` reset also clears
`usageTracker`, `lastUsage`, and the cache counters used by
`cacheEfficiencyText()`, using `newModel` and `startNewSession` as the main
entry points for locating the behavior.
---
Outside diff comments:
In `@internal/tui/model.go`:
- Around line 4232-4244: The `OnText` handler in `options.OnText` is stamping
`firstTokenAt` for any delta, which can record an early TTFT on whitespace-only
chunks. Update the `OnText` wrapper to mirror the `OnReasoning` behavior by only
setting `firstTokenAt` when the incoming `delta` contains non-whitespace text,
while keeping the existing `flushReasoning`, `m.sendAgentText`, and `onText`
callback flow intact.
---
Nitpick comments:
In `@internal/agent/system_prompt.go`:
- Around line 183-216: The skillsContext function can emit an empty
<available_skills> block when every Options.Skills entry has a blank Name,
because the loop skips all entries but still writes the header and footer. Add a
defensive guard in skillsContext after filtering/trimming names so it returns an
empty string when no valid skill names are present, ensuring the block is only
rendered when at least one skill like info.Name is actually included.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ef8d067e-8310-46af-910b-d1ebafca00bf
📒 Files selected for processing (28)
internal/agent/compaction.gointernal/agent/loop.gointernal/agent/partition_cache_stable_test.gointernal/agent/prompt_budget_test.gointernal/agent/skills_context_test.gointernal/agent/system_prompt.gointernal/agent/system_prompt.mdinternal/agent/system_prompt_models.gointernal/agent/system_prompt_models_test.gointernal/agent/system_prompt_test.gointernal/agent/types.gointernal/cli/app.gointernal/cli/exec.gointernal/cli/plugin_activate.gointernal/sessions/replay.gointernal/tui/command_polish_test.gointernal/tui/command_views.gointernal/tui/commands.gointernal/tui/flush_test.gointernal/tui/latency_test.gointernal/tui/model.gointernal/tui/model_test.gointernal/tui/new_session_test.gointernal/tui/session.gointernal/tui/session_controls.gointernal/tui/session_controls_test.gointernal/usage/cache_efficiency_test.gointernal/usage/tracker.go
Comment on lines
+191
to
+216
| for _, info := range options.Skills { | ||
| name := strings.TrimSpace(info.Name) | ||
| if name == "" { | ||
| continue | ||
| } | ||
| line := "- " + name | ||
| if desc := strings.TrimSpace(info.Description); desc != "" { | ||
| line += ": " + truncateForSkillLine(desc) | ||
| } | ||
| line += "\n" | ||
| // Always list at least one skill; past the budget, summarize the remainder | ||
| // as a count instead of silently dropping it. | ||
| if listed > 0 && spent+len(line) > skillsContextListBudget { | ||
| omitted++ | ||
| continue | ||
| } | ||
| b.WriteString(line) | ||
| spent += len(line) | ||
| listed++ | ||
| } | ||
| if omitted > 0 { | ||
| b.WriteString("- …and " + strconv.Itoa(omitted) + " more (call skill with a name; an unknown name lists them all)\n") | ||
| } | ||
| b.WriteString("</available_skills>") | ||
| return b.String() | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Sanitize embedded newlines in skill descriptions.
truncateForSkillLine/TrimSpace only handle leading/trailing whitespace. A Description containing internal newlines (common in multi-line frontmatter) will render as multiple raw lines inside a single - name: desc entry, fragmenting the <available_skills> list and confusing the model about how many skills exist.
🩹 Proposed fix: collapse internal whitespace
- if desc := strings.TrimSpace(info.Description); desc != "" {
+ if desc := strings.Join(strings.Fields(info.Description), " "); desc != "" {
line += ": " + truncateForSkillLine(desc)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| for _, info := range options.Skills { | |
| name := strings.TrimSpace(info.Name) | |
| if name == "" { | |
| continue | |
| } | |
| line := "- " + name | |
| if desc := strings.TrimSpace(info.Description); desc != "" { | |
| line += ": " + truncateForSkillLine(desc) | |
| } | |
| line += "\n" | |
| // Always list at least one skill; past the budget, summarize the remainder | |
| // as a count instead of silently dropping it. | |
| if listed > 0 && spent+len(line) > skillsContextListBudget { | |
| omitted++ | |
| continue | |
| } | |
| b.WriteString(line) | |
| spent += len(line) | |
| listed++ | |
| } | |
| if omitted > 0 { | |
| b.WriteString("- …and " + strconv.Itoa(omitted) + " more (call skill with a name; an unknown name lists them all)\n") | |
| } | |
| b.WriteString("</available_skills>") | |
| return b.String() | |
| } | |
| for _, info := range options.Skills { | |
| name := strings.TrimSpace(info.Name) | |
| if name == "" { | |
| continue | |
| } | |
| line := "- " + name | |
| if desc := strings.Join(strings.Fields(info.Description), " "); desc != "" { | |
| line += ": " + truncateForSkillLine(desc) | |
| } | |
| line += "\n" | |
| // Always list at least one skill; past the budget, summarize the remainder | |
| // as a count instead of silently dropping it. | |
| if listed > 0 && spent+len(line) > skillsContextListBudget { | |
| omitted++ | |
| continue | |
| } | |
| b.WriteString(line) | |
| spent += len(line) | |
| listed++ | |
| } | |
| if omitted > 0 { | |
| b.WriteString("- …and " + strconv.Itoa(omitted) + " more (call skill with a name; an unknown name lists them all)\n") | |
| } | |
| b.WriteString("</available_skills>") | |
| return b.String() |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/agent/system_prompt.go` around lines 191 - 216, The skill list
builder in system_prompt.go is still allowing internal newlines from
info.Description to flow into a single skill entry, which can break the
<available_skills> formatting. Update the logic around
buildAvailableSkills/available skills rendering so each description is
normalized to a single line before truncateForSkillLine is applied, collapsing
embedded whitespace/newlines in addition to the existing TrimSpace. Keep the
existing name/description handling and budget counting in place, but ensure each
emitted "- name: desc" line stays on one line.
Comment on lines
+1869
to
+1878
| // Roll the completed run's wall-time into the session's rolling average so | ||
| // /context can surface typical turn latency, not just token counts. | ||
| if msg.turnElapsed > 0 { | ||
| m.turnLatencySum += msg.turnElapsed | ||
| m.turnLatencyCount++ | ||
| } | ||
| if msg.ttft > 0 { | ||
| m.turnTTFTSum += msg.ttft | ||
| m.turnTTFTCount++ | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== internal/tui/model.go (around 1869-1878) ==\n'
sed -n '1848,1895p' internal/tui/model.go
printf '\n== search ttft / turnLatencyCount references ==\n'
rg -n "turnLatencyCount|turnLatencySum|ttft|avgTurnLatencyText" internal/tui -g '!**/*_test.go'
printf '\n== command_views.go relevant slice ==\n'
sed -n '430,490p' internal/tui/command_views.goRepository: Gitlawb/zero
Length of output: 6569
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1858,1888p' internal/tui/model.go
printf '\n---\n'
sed -n '450,475p' internal/tui/command_views.go
printf '\n---\n'
rg -n "ttft|turnLatencyCount|avgTurnLatencyText" internal/tuiRepository: Gitlawb/zero
Length of output: 4950
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== internal/tui/model.go around ttft definition ==\n'
sed -n '450,470p' internal/tui/model.go
printf '\n== internal/tui/model.go around agent response construction ==\n'
sed -n '4548,4602p' internal/tui/model.go
printf '\n== internal/tui/latency_test.go ==\n'
sed -n '1,120p' internal/tui/latency_test.goRepository: Gitlawb/zero
Length of output: 4540
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '450,470p' internal/tui/model.go
printf '\n---\n'
sed -n '4548,4602p' internal/tui/model.go
printf '\n---\n'
sed -n '1,120p' internal/tui/latency_test.goRepository: Gitlawb/zero
Length of output: 4398
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '4568,4598p' internal/tui/model.goRepository: Gitlawb/zero
Length of output: 1181
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "ttft := time.Duration|msg.err|return agentResponseMsg" internal/tui/model.go
sed -n '4550,4600p' internal/tui/model.goRepository: Gitlawb/zero
Length of output: 3940
Use the TTFT sample count in the label
avgTurnLatencyText prints the first-token average with turnLatencyCount, but turnTTFTCount is the actual sample size for that metric because ttft stays zero on tool-only or errored turns. That can overstate the TTFT turn count; use turnTTFTCount there or show both counts separately.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/tui/model.go` around lines 1869 - 1878, The TTFT label in the
session metrics is using the wrong sample count, so the average first-token
timing can report an inflated turn count. Update the `avgTurnLatencyText` logic
in `internal/tui/model.go` to use `turnTTFTCount` for the TTFT average display
(or present TTFT and latency counts separately if both are shown), while keeping
`turnLatencyCount` only for wall-time latency.
Comment on lines
+12
to
+33
| func TestStartNewSessionResetsState(t *testing.T) { | ||
| m := newModel(context.Background(), Options{}) | ||
| m.activeSession = sessions.Metadata{SessionID: "sess-old"} | ||
| m.sessionEvents = []sessions.Event{{Type: sessions.EventMessage}} | ||
| m.transcript = reduceTranscript(m.transcript, transcriptAction{kind: actionAppendUser, text: "hello"}) | ||
|
|
||
| next := m.startNewSession() | ||
|
|
||
| if next.activeSession.SessionID != "" { | ||
| t.Fatalf("expected active session id cleared, got %q", next.activeSession.SessionID) | ||
| } | ||
| if len(next.sessionEvents) != 0 { | ||
| t.Fatalf("expected session events cleared, got %d", len(next.sessionEvents)) | ||
| } | ||
| if len(next.transcript) != 2 || next.transcript[0].kind != rowWelcome { | ||
| t.Fatalf("expected transcript reset to welcome + note, got %#v", next.transcript) | ||
| } | ||
| // The note must name the prior session id so the user can /resume it. | ||
| if !transcriptContains(next.transcript, "sess-old") { | ||
| t.Fatalf("expected note to reference previous session id, got %#v", next.transcript) | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether usage/cache-tracker reset on /new is tested anywhere in the tui package.
rg -n -C3 'usageTracker|cacheEfficiency|Tracker' internal/tui/session.go internal/tui/model.go internal/tui/model_test.go internal/tui/new_session_test.go 2>/dev/nullRepository: Gitlawb/zero
Length of output: 2948
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the session reset implementation and related tests.
ast-grep outline internal/tui/session.go --view expanded || true
ast-grep outline internal/tui/new_session_test.go --view expanded || true
ast-grep outline internal/tui/latency_test.go --view expanded || true
echo "---- startNewSession / usage / cacheEfficiency references ----"
rg -n -C 3 'func \(.*startNewSession|cacheEfficiencyText|lastUsage|lastUsageSeen|usageTracker\.Reset|UsageTracker|cache hit|TTFT|latency' internal/tui/*.go
echo "---- test coverage references ----"
rg -n -C 3 'startNewSession|cacheEfficiencyText|lastUsage|lastUsageSeen|usageTracker|Reset\(\)' internal/tui/*_test.goRepository: Gitlawb/zero
Length of output: 16579
Add /new coverage for usage/cache reset. startNewSession() also clears usageTracker, lastUsage, and the counters behind cacheEfficiencyText(), but this test only checks session metadata and transcript state. Add an assertion here or in the latency test so the /new reset stays covered.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/tui/new_session_test.go` around lines 12 - 33, `startNewSession()`
is resetting additional state beyond session metadata and transcript, but
`TestStartNewSessionResetsState` only verifies the visible session fields.
Update this test (or the latency-related test that exercises the same flow) to
assert that the `/new` reset also clears `usageTracker`, `lastUsage`, and the
cache counters used by `cacheEfficiencyText()`, using `newModel` and
`startNewSession` as the main entry points for locating the behavior.
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.
Addresses post-launch user feedback comparing Zero against opencode on the same model (Minimax M3): larger system prompt, slower despite cache hits, skills not detected first-go, verbose responses, context filling in ~5 prompts, and missing
/new.Targets the high-impact, low-risk items. Defers the ones needing a maintainer decision (safety-policy rewrite, core-tool deferral activation semantics).
What's in this PR
Context economy
preserveLast 8→6,triggerRatio 0.8→0.7(both copies in sync).Response quality
defaultResponseStylebalanced→concise. Reversible via/style.Tool discovery
<available_skills>block in the system prompt lists installed skills (name + one-line description) so the model loads the right one on the first call instead of guessing and reading the failure. Budget-capped at 640 bytes with an "...and N more" fallback; renders nothing when no skills are installed (byte-identical to prior prompt).Session lifecycle
/newstarts a fresh session in place; previous session stays on disk and is named in the confirmation for/resume. Guards against mid-run reset./clearnow notes that context persists and points to/new.Performance & caching
partitionToolsno longer alpha-sorts the whole tools array every turn, which inserted a loaded deferred tool mid-array and shifted the cached prefix on every load. It now builds append-only regions[eager sorted] + [tool_search] + [loaded deferred sorted]so a mid-session load grows the tail instead of busting the eager prefix. Regression test asserts the eager block precedes any loaded tool./contextnow shows cache hit rate (flags low rate over >5 turns as a possible unstable prefix) and rolling average turn latency with time-to-first-token. Both reset on/new.Measured baselines
Auto-mode, no workspace: base prompt 3162 tokens, eager tool schemas 2433 across 12 tools. Budget ceilings set ~10% above.
Verification
go build ./...cleango veton all changed packages cleaninternal/agent,internal/tui,internal/usage,internal/cli,internal/sessionsfull test suites greenDeliberately deferred (need a decision)
These were intentionally left out of this PR and need a maintainer call before implementation:
git reset --hard, single-filerm, non-network package managers,git push,sudointent), so a naive cut is a real safety regression. The inverted approach (keep enumeration for uncaught rules, compress the caught ones) needs sign-off.web_fetchdeferral — there's an existing tested invariant that core tools are never deferral-eligible, because the eligible count gates whentool_searchregisters at all. Making a core toolDeferred()changes when deferral activates, not just per-tool exposure. Needs a design decision + a rework ofdeferredEligibleCount.Feedback points addressed
/newcommand missing/clearclarified<available_skills>blockSummary by CodeRabbit
New Features
/newcommand to start a fresh session.Bug Fixes
/clearand/new.Style