Skip to content

Remove chat command + add CI coverage gate (P0) - #81

Merged
marcusolsson merged 1 commit into
mainfrom
fix/p0-remove-chat-coverage-gate
Jun 29, 2026
Merged

Remove chat command + add CI coverage gate (P0)#81
marcusolsson merged 1 commit into
mainfrom
fix/p0-remove-chat-coverage-gate

Conversation

@marcusolsson

Copy link
Copy Markdown
Contributor

What

Addresses the two P0 items from the code-quality review.

1. Remove the chat command

The chat command advertised three conflicting default models in a single request flow:

  • kimi-k2.6 in the --help text
  • openai/gpt-oss in buildCompletionOptions
  • google/gemma-3-27b-it as the service fallback

It was also the largest command (524 lines) at 1.21% line coverage. Per product direction, the command is removed entirely rather than fixing the default.

Deleted: commands/chat.ts, services/chat-service.ts (+ tests), utils/markdown-renderer.ts (used only by chat), and the now-orphaned marked / marked-terminal runtime deps + @types/* dev deps. CHAT entries stripped from command-structure.ts.

Kept: DefaultApiKeyManager — still wired into auth/credentials/api-key.ts and the api-keys set-default/get-default subcommands. (Its "for chat commands" copy is now slightly stale; left for a follow-up.)

2. Add a CI coverage gate

@vitest/coverage-v8 was installed but never run in CI; coverage could regress silently. Now:

  • vitest.config.ts: v8 coverage over all src files (excluding generated types/schemas/__tests__), thresholds ratcheted just below the post-removal baseline.
  • package.json: test:coverage script.
  • test.yml: CI runs npm run test:coverage (enforces thresholds) instead of test:run.
  • .gitignore: ignore coverage/ (artifacts were previously committed by accident; cleaned up here).

Post-removal coverage baseline

All source files (40): 52.36% stmt / 54.72% branch / 50.72% func / 52.47% line — up from ~60% on the test-imported set, and the chat removal eliminated the worst drag on the average. Thresholds set at 50/52/48/50 with a small anti-flake margin.

Verification

All gates green locally:

  • npm run typecheck
  • npm run lint
  • npm run format:check
  • npm run build
  • npm run test:coverage ✅ (259 tests, thresholds met)

User-facing note: removing a published command is intentional; flag for the next release notes.

Plan: PLAN-P0.md (kept out of this PR's scope).

The chat command advertised three conflicting default models (kimi-k2.6
in help text, openai/gpt-oss in the option builder, google/gemma-3-27b-it
in the service) and was effectively untested (1.21% line coverage).
Remove the command entirely per product direction.

Removed:
- src/commands/chat.ts, src/services/chat-service.ts (+ their tests)
- src/utils/markdown-renderer.ts (only used by chat)
- marked, marked-terminal runtime deps + @types/* dev deps (orphaned)
- CHAT entries from command-structure constants
- Chat Command section from README

Kept DefaultApiKeyManager: still used by auth/credentials/api-key.ts and
the api-keys set-default/get-default subcommands.

Add a CI coverage gate to prevent silent coverage regressions:
- vitest.config.ts: v8 coverage, include all src, exclude generated types/
  schemas/__tests__, thresholds ratcheted just below the post-removal
  baseline (50/52/48/50 stmt/branch/func/line)
- package.json: test:coverage script
- test.yml: run npm run test:coverage in CI (replaces plain test:run)
- .gitignore: ignore coverage/ (artifacts were previously committed)
@marcusolsson
marcusolsson merged commit 734fc6c into main Jun 29, 2026
1 check passed
@marcusolsson
marcusolsson deleted the fix/p0-remove-chat-coverage-gate branch June 29, 2026 11:51
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