mirror of
https://github.com/himanshudongre/smriti.git
synced 2026-08-28 05:14:59 +00:00
A fresh Smriti install with no provider configured could still run `smriti checkpoint create --extract`, which silently returned MockAdapter content like "Mock decision from provider". If committed, that placeholder text became part of the user's real reasoning state. Root cause: backend/app/api/routes/checkpoint.py:417 called `get_adapter(cfg.background.provider, allow_mock=True)`. The `allow_mock=True` flag means the registry quietly returns MockAdapter when no API key is configured, rather than raising. The CLI received the canned mock JSON and committed it without inspecting whether it came from a real LLM. The extract endpoint was the only route in the codebase with this pattern — draft, review, chat title, and chat send already correctly pass `allow_mock=False`. The new contract: - Core Smriti (setup, doctor, quickstart, state/current/metrics, claims, attach, manual JSON checkpoints) requires no API key. - Real LLM-backed paths (`--extract`, draft, review, chat send) require a configured provider — OpenAI / Anthropic / OpenRouter / generic OpenAI-compatible (local models like Ollama). - Mock extraction still works for tests and demos, but only when the caller explicitly opts in (use_mock=true on the HTTP payload). It is never silently the default. Backend: - POST /api/v5/checkpoint/extract now passes allow_mock=False and catches ProviderNotConfiguredError, returning HTTP 412 with a structured detail: error code, human message, the provider it tried, and a list of fix paths the CLI surfaces. - CheckpointExtractResponse gains `provider` and `model` echo fields (additive, default empty) so callers can confirm what answered. CLI: - checkpoint create --extract catches 412 and prints the actionable fix list; exits 78 (EX_CONFIG). Defense in depth: even on a 200, refuses to commit if response.provider == "mock" on the default path (so any future regression in the backend is still caught). - smriti doctor surfaces background provider state prominently: `ready (real LLM extraction enabled)` or `⚠ MOCK or DISABLED — … will fail until a provider is configured`. - smriti doctor --strict exits 78 when the background provider is mock/disabled or the backend is unreachable. Safe to wire into CI before any --extract step. - On a successful --extract commit, the CLI shows `extracted via <provider>/<model>` under the commit confirmation. Docs: - README: new "Provider configuration (LLM-backed features)" section drawing the boundary explicitly; mentions the generic provider for local OpenAI-compatible models; flags mock as test-only. - .env.example: rewrote the provider section so an empty key or a model-without-a-key is not interpreted as "ready". - Skill pack template: new §4.1 "Before your first --extract: verify the provider" telling agents to run `smriti doctor`, refuse --extract when background_provider is mock/disabled, and fall back to manual JSON checkpoints or ask the human to configure a provider. Re-rendered to AGENTS.md (Codex target). The Claude Code target (.claude/skills/smriti/SKILL.md) is gitignored per-user install; rerun `smriti skills install claude-code` to refresh. - website/index.html: Try-it lede now spells out which features need a provider rather than gesturing at "optional LLM features". Tests: - test_extract_without_provider_fails_loud: regression for the bug — monkeypatches get_adapter to raise ProviderNotConfiguredError, asserts HTTP 412 with the structured detail shape, and asserts the response body contains neither "Mock decision from provider" nor "Mock Checkpoint". This test would fail on pre-fix code. - test_extract_with_provider_echoes_provider_and_model: pins the green path — provider and model must be echoed and must not be "mock" when the real adapter answers. - test_extract_happy_path_with_mock: unchanged, still pins the explicit use_mock=true contract. - Full backend integration suite: 165 passed locally (with the pre-existing real-provider draft test passing under backend/config/providers.yaml).
183 lines
6.9 KiB
Python
183 lines
6.9 KiB
Python
"""Integration tests for POST /api/v5/checkpoint/extract.
|
|
|
|
Post launch-blocker fix (provider-extract-safety):
|
|
|
|
- The DEFAULT path (no `use_mock` flag) requires a real configured
|
|
background provider. If none is configured, the endpoint returns
|
|
HTTP 412 with a structured `provider_not_configured` detail. It
|
|
never silently falls back to MockAdapter — that would pollute a
|
|
real user's reasoning state with placeholder content.
|
|
|
|
- The EXPLICIT mock path (`use_mock=True`) still works for tests
|
|
and demos. It always returns the canned MockAdapter response.
|
|
|
|
These tests pin both contracts.
|
|
"""
|
|
|
|
import json
|
|
import pytest
|
|
|
|
from app.config_loader import ProviderNotConfiguredError
|
|
from app.providers.base import ProviderAdapter
|
|
from app.providers import registry as provider_registry
|
|
from app.api.routes import checkpoint as checkpoint_route
|
|
|
|
|
|
def _sample_markdown() -> str:
|
|
return """# Design: envdiff CLI
|
|
|
|
## Objective
|
|
Build a stdlib-only CLI that compares two .env files.
|
|
|
|
## Decisions
|
|
- Use argparse, not click
|
|
- Single file, not a package
|
|
|
|
## Assumptions
|
|
- Python 3.11+ is available
|
|
|
|
```python
|
|
def main():
|
|
print("hello")
|
|
```
|
|
"""
|
|
|
|
|
|
# ── Explicit mock path: use_mock=True ─────────────────────────────────────────
|
|
|
|
|
|
def test_extract_happy_path_with_mock(client):
|
|
"""use_mock=True: MockAdapter returns canned content, 200 OK."""
|
|
r = client.post(
|
|
"/api/v5/checkpoint/extract",
|
|
json={"content": _sample_markdown(), "use_mock": True},
|
|
)
|
|
assert r.status_code == 200, r.text
|
|
data = r.json()
|
|
# MockAdapter JSON mode returns the canned _MOCK_JSON_RESPONSE in
|
|
# registry.py — these are the exact values defined there.
|
|
assert data["title"] == "Mock Checkpoint"
|
|
assert data["summary"].startswith("Mock summary")
|
|
assert "Mock decision from provider" in data["decisions"]
|
|
assert "Mock assumption from provider" in data["assumptions"]
|
|
# Tasks are now structured objects; a plain string from the mock
|
|
# becomes {"text": "Mock task from provider"}.
|
|
assert any(
|
|
(t.get("text") if isinstance(t, dict) else t) == "Mock task from provider"
|
|
for t in data["tasks"]
|
|
)
|
|
assert "Mock open question from provider" in data["open_questions"]
|
|
assert "MockEntity" in data["entities"]
|
|
assert len(data["artifacts"]) == 1
|
|
assert data["artifacts"][0]["label"] == "Mock artifact"
|
|
# Echo fields must mark this as mock so callers can refuse to commit.
|
|
assert data["provider"] == "mock"
|
|
assert data["model"] == "mock"
|
|
|
|
|
|
# ── Default path without provider: must fail loud (the launch-blocker fix) ───
|
|
|
|
|
|
def test_extract_without_provider_fails_loud(client, monkeypatch):
|
|
"""Default path (no use_mock) + no real provider configured: HTTP 412 with
|
|
a structured 'provider_not_configured' detail. Never silent mock.
|
|
|
|
This is the regression test for the launch-blocker bug where the extract
|
|
endpoint silently returned MockAdapter content like
|
|
"Mock decision from provider" when no provider was configured.
|
|
"""
|
|
# Force the provider-not-configured state regardless of test-env API keys.
|
|
def _raise_not_configured(provider: str, allow_mock: bool = False):
|
|
# The endpoint must call with allow_mock=False (no silent fallback).
|
|
assert allow_mock is False, (
|
|
"extract endpoint must call get_adapter with allow_mock=False"
|
|
)
|
|
raise ProviderNotConfiguredError(f"no API key for {provider}")
|
|
|
|
monkeypatch.setattr(checkpoint_route, "get_adapter", _raise_not_configured)
|
|
|
|
r = client.post(
|
|
"/api/v5/checkpoint/extract",
|
|
json={"content": _sample_markdown()},
|
|
)
|
|
assert r.status_code == 412, r.text
|
|
detail = r.json().get("detail")
|
|
assert isinstance(detail, dict), f"detail must be a structured dict, got {type(detail)}"
|
|
assert detail.get("error") == "provider_not_configured"
|
|
assert "not configured" in detail.get("message", "").lower()
|
|
assert detail.get("provider") # backend echoes which provider it tried
|
|
assert isinstance(detail.get("fix"), list)
|
|
assert len(detail["fix"]) >= 3, "fix list should enumerate at least 3 paths"
|
|
# CRITICAL: response body must NOT contain mock content. The bug we are
|
|
# fixing literally returned "Mock decision from provider" in the response.
|
|
body_text = r.text
|
|
assert "Mock decision from provider" not in body_text
|
|
assert "Mock Checkpoint" not in body_text
|
|
|
|
|
|
# ── Default path with a real provider: provider/model echoed ─────────────────
|
|
|
|
|
|
class _FakeAdapter(ProviderAdapter):
|
|
"""Test double for a real provider — returns valid JSON shaped like a
|
|
real extraction so we can verify the endpoint's success-path metadata."""
|
|
|
|
def send(self, messages, model, **kwargs):
|
|
return json.dumps({
|
|
"title": "Real Extraction Result",
|
|
"objective": "Build envdiff CLI.",
|
|
"summary": "A short summary of the design doc.",
|
|
"decisions": ["Use argparse, not click"],
|
|
"assumptions": ["Python 3.11+ is available"],
|
|
"tasks": [{"id": "impl-1", "text": "Scaffold the CLI"}],
|
|
"open_questions": [],
|
|
"entities": ["envdiff"],
|
|
"artifacts": [],
|
|
})
|
|
|
|
def healthcheck(self) -> bool: # pragma: no cover
|
|
return True
|
|
|
|
|
|
def test_extract_with_provider_echoes_provider_and_model(client, monkeypatch):
|
|
"""Default path + a real (faked) provider: 200 OK, response echoes
|
|
`provider` and `model` so the CLI can show 'extracted via openai/gpt-4o-mini'
|
|
on commit and refuse to persist mock content."""
|
|
def _fake_adapter(provider: str, allow_mock: bool = False):
|
|
return _FakeAdapter()
|
|
|
|
monkeypatch.setattr(checkpoint_route, "get_adapter", _fake_adapter)
|
|
|
|
r = client.post(
|
|
"/api/v5/checkpoint/extract",
|
|
json={"content": _sample_markdown()},
|
|
)
|
|
assert r.status_code == 200, r.text
|
|
data = r.json()
|
|
assert data["title"] == "Real Extraction Result"
|
|
# Echo: must be the configured provider/model, NOT "mock".
|
|
assert data["provider"] != "mock", "real-provider path must not echo provider=mock"
|
|
assert data["provider"] != ""
|
|
assert data["model"] != "mock"
|
|
assert data["model"] != ""
|
|
|
|
|
|
# ── Validation tests (unchanged) ─────────────────────────────────────────────
|
|
|
|
|
|
def test_extract_rejects_empty_content(client):
|
|
"""Empty content is a 422 validation error."""
|
|
r = client.post(
|
|
"/api/v5/checkpoint/extract",
|
|
json={"content": " "},
|
|
)
|
|
assert r.status_code == 422, r.text
|
|
|
|
|
|
def test_extract_rejects_oversized_content(client):
|
|
"""Content exceeding 200000 character cap is a 422 validation error."""
|
|
r = client.post(
|
|
"/api/v5/checkpoint/extract",
|
|
json={"content": "x" * 300_000},
|
|
)
|
|
assert r.status_code == 422, r.text
|