Expose SentryQuery over MCP, with cost controls and container packaging - #15
Merged
Conversation
Adds mcp_server.py, an interface layer over the existing Researcher -> Critic graph. It registers one MCP tool, ask_corpus(question), which calls the existing run_pipeline rather than reimplementing any of it, and returns the full structured result: answer, sources, tool_used, confidence, plus the Critic's verdict and reason, so a client sees the groundedness ruling and not just answer text. The compiled graph is built lazily on first call, which keeps the module importable without live keys and makes it testable offline. Keys are read from the same .env config.py already loads, and none are read or stored here. Pins the official MCP SDK at mcp==2.0.0 in requirements.txt. The pin is deliberate: the 2.x server API (MCPServer) is not the 1.x one (FastMCP), so an unpinned upgrade would break this module.
Every uncached query costs a paid OpenAI embedding plus a Pinecone query, and an MCP server is meant to be left running, so exposing ask_corpus without ceilings would not be safe to demo. Two controls, both real: A bounded in-process answer cache keyed on the normalized question (trimmed, lowercased, whitespace collapsed), so " What Were TOTAL sales? " and "what were total sales?" are one entry rather than two paid runs. It holds 128 entries by default and evicts the oldest by insertion at the cap, overridable with SENTRYQUERY_MCP_CACHE_SIZE. A per-client sliding-window rate limiter over a rolling 60 seconds, defaulting to 10 ask_corpus calls per minute and overridable with SENTRYQUERY_MCP_RATE_LIMIT. Over-limit calls raise RateLimitExceeded with the limit, the retry delay, and the override name, before any paid call starts. The cache is checked before the limiter, so a repeat question costs nothing and spends no rate-limit budget. Clients are keyed by MCP session, falling back to a single key over stdio where one process serves one client. Both controls are in-process only: they reset on restart and are not shared between processes. Documents both env vars and their defaults in .env.example.
Adds a Dockerfile on python:3.11-slim, a .dockerignore, and a docker-compose.yml covering all three run modes from one image. The Dockerfile copies requirements.txt and installs before any source, so editing a .py file reuses the dependency layer instead of triggering a full reinstall. Runtime deps only: requirements-dev.txt stays out of the image. It creates a non-root user and runs as it, exposes 8501, and adds a HEALTHCHECK against Streamlit's own health endpoint. The default command serves the UI on 8501 bound to 0.0.0.0 so the published port reaches it. No secret reaches any layer. There is no ARG or ENV for a key, .dockerignore excludes .env, and compose injects keys at run time with env_file. ./docs is bind-mounted read-only rather than baked in, so PDFs can be swapped and re-ingested without a rebuild. Compose exposes the UI as the default service and puts ingest and the MCP server behind a tools profile, since both are one-shot commands rather than long-running services. The image healthcheck probes Streamlit, so it is disabled on those two. Invocations for all three modes are documented at the top of the file, including the docker run form an MCP client needs when it owns the stdio pipe itself. Docker is deliberately not added to the CI workflow. A cold docker build has no layer cache on a fresh runner and would reinstall the full runtime stack, which would push the job past its current sub-two-minute budget for no deterministic signal the offline tests do not already give.
Adds tests/test_mcp_server.py, 12 tests that stub run_pipeline and get_system so nothing here builds a graph, needs a key, or makes a live call. They cover the three things CI can honestly verify: - the cache serves a repeated question and does not invoke run_pipeline a second time, including across differing case and whitespace, plus oldest-first eviction at the cap - the rate limiter rejects an over-limit call without invoking run_pipeline, is per client, and slides its window, and cache hits spend no budget - the ask_corpus response carries the AnswerSchema fields (answer, sources, tool_used, confidence) plus the Critic's verdict, reason, and revision count, with the injected context parameter kept out of the public tool schema Writing them caught a real defect: AnswerCache copied entries shallowly, so a caller mutating a returned response also mutated the cached sources list. Now deep-copied on the way in and out. Extends the existing CI workflow's scope comment rather than its steps. The ruff and pytest steps are unchanged and pytest already collects the new file, so the job still runs with no secrets and well inside two minutes. The comment now says explicitly what CI does and does not earn: it verifies the MCP wiring, the cache, and the rate limiter deterministically, and it does not verify the Critic's groundedness judgment, which needs live keys and stays in the local eval run.
Adds an "MCP server" section to the README covering the ask_corpus tool and every field it returns, how to register the server with an MCP client, and the two cost controls with their defaults (128 cache entries, 10 calls per minute per client) and their env-var overrides. States that the user supplies their own OpenAI and Pinecone keys via .env, that no key is stored or logged, and that both controls are in-process only rather than a distributed cache or quota. Adds a "Running with Docker" section covering all three run modes, the docker run form an MCP client needs when it owns the stdio pipe, and the fact that keys are injected at run time and nothing is deployed or hosted. Tightens the CI section to match what the suite now earns: it covers the MCP wiring, the cache, and the rate limiter deterministically, and it does not run the Critic's groundedness judgment or the live model. Also records why Docker is not in the workflow. Adds the MCP SDK and Docker to the Stack list. The resume-grounding notes were updated alongside this, but that file is gitignored and stays local, so it is not part of this commit.
The repo venv on my machine had been copied by folder sync, which left its activate script and about fifteen console-script shebangs still pointing at the path the venv was originally created under. Activating it silently activated the other environment, so a bare `pip install` landed somewhere other than where `python` would look. That is exactly the failure mode that ends in an ImportError on a module the user just installed. Switches the documented install to `python -m pip install`, which always resolves to the interpreter that was just activated regardless of what any stale pip script points at, and explains why. Adds a one-line import check so the environment is confirmed working before the user gets as far as running the app. Repairing my local venv is not part of this commit; venv/ is gitignored.
…e server An MCP client starts the server process itself, from whatever working directory the client happens to have. Claude Desktop uses /. The bare load_dotenv() in config.py searches upward from the cwd, so it never found this repo's .env and the server died on import with a Pinecone missing-key error. Every check so far had passed because I always launched it from the repo root myself, which is the one directory where the bug is invisible. Anchors the lookup to the module's own directory instead. That has to run before graph is imported, since graph imports config and config builds the Pinecone client at import time, so the import order carries a deliberate noqa for the E402 the project's ruff config would otherwise raise. load_dotenv does not override variables already in the environment, so a key injected by the client still takes precedence over the file. Reproduced from cwd=/ before the fix and confirmed working from /tmp after. The alternative, copying keys into the client config, would have spread secrets into a second plaintext file for no benefit. Also widens the .gitignore entry for my untracked resume-grounding notes from a literal .txt to SentryQuery_project_context.*, since I renamed that file to .md and the old pattern would have left it committable.
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.
Adds a third way to run SentryQuery: as an MCP server, so any MCP client can query the corpus through a standard tool interface. This is an interface, cost-control, and packaging layer only. No agent, graph, Critic, or eval logic changed.
MCP server
mcp_server.pyexposes one tool,ask_corpus(question), which calls the existingrun_pipelineand returns the full structured result: theAnswerSchemafields plus the Critic's verdict, reason, and revision count. A client sees the groundedness ruling, not just answer text.The graph is built lazily, so the module imports without keys and stays testable offline. The MCP SDK is pinned at
mcp==2.0.0because the 2.x server API (MCPServer) is not the 1.x one (FastMCP), and an unpinned upgrade would break the module silently.Cost controls
Every uncached query costs a paid OpenAI embedding plus a Pinecone query, and an MCP server is meant to be left running. Two ceilings, both real and both covered by tests:
SENTRYQUERY_MCP_CACHE_SIZESENTRYQUERY_MCP_RATE_LIMIT, rejecting over-limit calls before any paid call startsThe cache is checked before the limiter, so a repeated question costs nothing and spends no rate-limit budget. Both are in-process only: they reset on restart and are not shared between processes.
Container
One image serves all three run modes. The dependency layer installs before source is copied, so a code edit does not trigger a reinstall. Runtime dependencies only, non-root user, HEALTHCHECK against Streamlit's health endpoint, and no secret in any layer: keys are injected at run time via
env_fileand./docsis bind-mounted read-only so PDFs swap without a rebuild.Tests and honest scope
12 new offline tests, 34 total, all passing with no API keys. They cover the cache, the rate limiter, and the tool response shape with
run_pipelinestubbed. Writing them caught a real defect:AnswerCachecopied entries shallowly, so a caller mutating a returned response also mutated the cached sources list.CI covers the MCP wiring, the cache, and the rate limiter deterministically. It does not run the Critic's groundedness judgment or the live model, which still need real keys and the local eval run. Docker is deliberately not in CI: a cold build has no layer cache on a fresh runner and would push the job past its sub-two-minute budget for no signal the offline tests do not already give.
Verification
.envor key in any layer, pip layer stays cached across a source edit, and the container serves/_stcore/healthwith HTTP 200 on 8501ask_corpus, and got backtool_used=docs,n_sources=4,verdict=APPROVE,confidence=0.95Notes
Two bugs surfaced only because the server is started by someone else:
config.pyloads.envrelative to the working directory, but an MCP client launches the server from its own directory (Claude Desktop uses/), so the server died on import. The lookup is now anchored to the module path. It had passed every earlier check because I always launched it from the repo root myself, the one directory where the bug is invisible.pip installstep is nowpython -m pip install, which installs into the interpreter that was actually activated rather than whatever a stalepipscript points at.