Skip to content

fix(providers): cut per-call cost of CLI-provider scans - #808

Open
rng1995 wants to merge 23 commits into
mainfrom
fix/claude-cli-session-cost
Open

rng1995 wants to merge 23 commits into
mainfrom
fix/claude-cli-session-cost

Conversation

@rng1995

@rng1995 rng1995 commented Oct 8, 2026

Copy link
Copy Markdown
Collaborator

Why

A user on r/ClaudeAI (thread) asked Claude Code to scan their 15 skills with SkillSpector. The agent picked claude_cli on its own and ran five scans in parallel. Because every LLM call starts a separate claude -p session, that came to 434 sessions in about 6 minutes, 22.4M tokens (about 34k of startup context per session, on Opus), and 98% of their 5-hour plan window. It also left 1,144 folders in ~/.claude/projects.

#298 (for #295) already keeps MCP servers, hooks, and settings out of the child. Three costs were still left: the built-in tool schemas, Claude Code's default agent system prompt, and a persisted transcript for every call.

Changes

  1. Lower per-call cost (claude argv). Add --tools "", so the built-in tool definitions are no longer sent. --allowed-tools "" only blocks tool use (CLI reference). Also replace the default system prompt with a short fixed one via --system-prompt; the analyzer instructions still arrive on stdin.
  2. No session clutter. Add --no-session-persistence. Each call runs in a fresh temp cwd, so before this change every call left its own project folder.
  3. Smaller default fan-out and a warning. When SKILLSPECTOR_MAX_LLM_CONCURRENCY is unset and the active provider is a CLI provider, the default is now 2 instead of 10. Once per process, when a CLI provider is available, SkillSpector warns that every LLM call is a separate session on the user's plan and points to --no-llm.
  4. skill-inspector guidance. Keep --no-llm unless the user asks for LLM analysis. Never choose or change SKILLSPECTOR_PROVIDER without asking. Don't run CLI-provider scans in parallel.

Not in this PR

  • Printing the expected number of LLM calls before a scan, or asking the user to confirm. The call count isn't known up front, and a prompt would break non-interactive runs. The warning plus the lower default cover the immediate risk.
  • A README section on CLI-provider cost.

Testing

  • uv run python -m pytest tests/unit/test_agent_cli.py tests/unit/test_providers.py tests/nodes/test_llm_analyzer_base.py: 539 passed, 10 skipped. New tests cover the argv flags, the CLI concurrency default (and that an explicit value still wins), and the warn-once behaviour.
  • ruff check and ruff format --check are clean.
  • The new flags were checked against the current Claude Code CLI reference, not run against a live claude binary. Before merge, a maintainer with claude installed should run one SKILLSPECTOR_PROVIDER=claude_cli scan to confirm that auth still works and the output parses. Older claude builds that lack --tools or --no-session-persistence will fail closed with a non-zero exit.

🤖 Generated with Claude Code

rng1995 and others added 13 commits October 8, 2026 12:05
Each LLM call with claude_cli starts a separate `claude -p` session. A
user scanning 15 skills (five scans in parallel) started 434 sessions in
about 6 minutes and used 98% of their 5-hour plan window, at roughly 34k
tokens of startup context per session.

- claude argv: add `--tools ""` so built-in tool schemas are not sent
  (`--allowed-tools ""` only denies use), replace the default agent
  system prompt with a short fixed one, and pass
  `--no-session-persistence` so calls stop leaving one
  ~/.claude/projects folder each.
- Default LLM fan-out to 2 for CLI providers when
  SKILLSPECTOR_MAX_LLM_CONCURRENCY is unset (HTTP providers keep 10).
- Warn once per process when a CLI provider is available that every LLM
  call is a separate session on the user's plan, and point to --no-llm.
- skill-inspector: keep --no-llm unless the user asks, never pick a
  provider on the agent's own, and don't run CLI-provider scans in
  parallel.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Narendran Raghavan <nraghavan@nvidia.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Narendran Raghavan <nraghavan@nvidia.com>
@mohgupta-ship-it
mohgupta-ship-it self-requested a review October 11, 2026 00:23
@mohgupta-ship-it

Copy link
Copy Markdown
Member

Thanks for proactively working on this, will review this on monday.

@rng1995 rng1995 left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[SkillSpector Review]

@rng1995 thank you for turning a real user's cost report into a focused fix with clearly stated limits.

This PR is authored by the review bot account, so this review can only COMMENT. A human maintainer must review and approve (or request changes).

Value and readiness

The problem is real: per-call startup context, persisted sessions, and a default 10-way fan-out billed to the user's plan. The argv changes and the lower CLI default are coherent, and an explicit SKILLSPECTOR_MAX_LLM_CONCURRENCY still wins. It is not ready to approve: the cost warning fires after the cost it warns about, and the new claude flags have not been run against a real binary.

Material findings

  1. Blocker, src/skillspector/providers/_agent_cli_base.py:82: the warning is emitted from is_available(), but skillspector scan never calls that before the fan-out. On the scan path the only caller is the report node (nodes/report.py:1819, and _build_metadata at :1264, via is_llm_available()), which runs after every semantic analyzer and meta_analyzer. A CLI-provider scan therefore prints the warning only after the sessions have run, contrary to the docstring ("before any LLM call fans out") and the PR description. The MCP server probes up front (mcp_server.py:145), so it warns in time there; and since #806 (now merged into this branch) --no-llm scans no longer probe, so they correctly stay silent. Please warn where CLI-provider LLM work actually starts, for example in get_chat_model() when it builds an AgentCLIChatModel (llm_utils.py:616-618) or at the start of arun_batches_detailed when the active provider is a CLI provider, and add a test that the warning is logged before the first complete() call of a scan.
  2. Blocker (verification), src/skillspector/providers/_agent_cli.py:285-289: --tools "", --system-prompt and --no-session-persistence were checked only against the CLI reference. Replacing Claude Code's system prompt is the change most likely to interact with subscription (OAuth) logins, the main claude_cli auth path. claude_cli also has no version gate: an older claude that lacks a flag passes claude auth status, and then every call fails, so semantic coverage is reported as degraded (honest, but total). As the description asks, a maintainer with claude installed should run one SKILLSPECTOR_PROVIDER=claude_cli scan, ideally with a subscription login, and confirm auth, output parsing, and that no ~/.claude/projects folders appear.

PIC tradeoffs

  • Coverage vs. cost: the scan's 600 s workflow budget (DEFAULT_MAX_WORKFLOW_SECONDS, state.py:46) also bounds the semantic analyzers. At a 2-way fan-out the LLM phase of a CLI scan can take up to ~5x longer, so larger skills will more often end with runtime_limit gaps in semantic coverage. Those gaps are reported honestly, but the default now favors cost over coverage. The warning could also mention SKILLSPECTOR_MAX_WORKFLOW_SECONDS.
  • The skill-inspector rule (keep --no-llm unless the user asks) is a sensible default, but it changes product behavior for agent-driven scans.

Verification and gaps

  • Read the full diff at 8fdefc92. Its own change has the same patch-id as at 9881bafc (cfd4e64a + fb2b70d2); the new head only merges main, including #806.
  • Traced every is_available() / is_llm_available() caller on the scan and MCP paths; resolve_max_concurrency through arun_batches_detailed and _shared_limiter; and get_active_provider selection (cheap, no probing).
  • #806 does not touch _agent_cli*.py or llm_analyzer_base.py; its report and MCP changes narrow finding 1 to timing.
  • Not run: tests or a claude binary (none is installed here). CI on the head is action_required; the last completed run (e47a7d4c) was green.

Comment thread src/skillspector/providers/_agent_cli_base.py Outdated
github-actions Bot and others added 3 commits October 14, 2026 02:48
Signed-off-by: Narendran Raghavan <nraghavan@nvidia.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
skillspector scan only probes is_available() from the report node, after every
semantic analyzer has run, so the session-cost warning arrived after the cost.
get_chat_model() now warns (once per process) when it builds a CLI-provider
chat model, before the analyzers fan out. The MCP server's up-front probe still
warns through is_available().

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Narendran Raghavan <nraghavan@nvidia.com>
@rng1995

rng1995 commented Oct 14, 2026

Copy link
Copy Markdown
Collaborator Author

Follow-up on the two review blockers, plus the merge with main:

1. The cost warning arrived after the cost. Fixed in 0f51020; see the review thread for details.

2. Checking the new claude flags against a real binary (Claude Code 2.1.293):

  • claude --help lists all three flags: --tools ("" disables every built-in tool), --system-prompt, and --no-session-persistence (print mode only, and SkillSpector always runs -p).
  • I ran SkillSpector's exact _build_claude_argv() against that binary. It passed argument parsing and stopped at Not logged in · Please run /login. As a control, the same argv with an unknown flag fails at once with error: unknown option.
  • Still not verified: a logged-in run, meaning auth with a subscription login, parsing of the response, and confirming that no ~/.claude/projects folders are created. The binary I tested isn't logged in. Before approving, please run one scan with a logged-in claude:
    SKILLSPECTOR_PROVIDER=claude_cli skillspector scan <small skill>

Merge with main: the only conflict was in CHANGELOG.md, and I kept both entries (#799 and this PR).

Tests:

  • ruff check and ruff format --check are clean.
  • tests/unit/test_agent_cli.py, tests/unit/test_providers.py, tests/unit/test_llm_utils.py and tests/nodes/test_llm_analyzer_base.py: 614 passed, 10 skipped.
  • Full tests/unit and tests/nodes run locally: 138 failures, the same set as an untouched main checkout gives in this environment. They are order-dependent (mcp_result["llm_used"] assertions), and every failing file passes when run on its own. This PR introduces none of them.

@rng1995

rng1995 commented Oct 14, 2026

Copy link
Copy Markdown
Collaborator Author

Live claude_cli check at 0f51020. This closes the remaining verification item from the review.

Setup: Claude Code 2.1.293, signed in with a claude.ai subscription login (authMethod: claude.ai). I scanned tests/fixtures/mcp_clean_skill with SKILLSPECTOR_PROVIDER=claude_cli.

Results:

  • Auth and transport: llm_calls_attempted: 5 and llm_calls_succeeded: 5, with effective_adapter: claude_cli. Replacing the system prompt with --system-prompt and disabling tools with --tools "" both work with a subscription login.
  • Output parsing: semantic_developer_intent, semantic_quality_policy, semantic_security_discovery and meta_analyzer all finished completed. analysis_completeness.is_complete is true, and the clean fixture's verdict is SAFE.
  • No session clutter: ~/.claude/projects had 8 folders before the scan and 8 after, so --no-session-persistence holds.
  • Cost warning: it was logged once, during the run, from _agent_cli_base, not at the end of the report.

Unrelated noise, not from this PR: CLI providers also log Model '' ... not found in model_registry.yaml for each slot. This PR doesn't touch constants.py or model_info.py.

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.

2 participants