diff --git a/.dockerignore b/.dockerignore index 9ca4953e1..8cffe401b 100644 --- a/.dockerignore +++ b/.dockerignore @@ -1,11 +1,10 @@ -# The root Dockerfile builds only server/, so exclude the whole repo and add that one -# subtree back. An allow-list keeps the context minimal without needing an update every -# time the repo grows a new top-level directory. +# The root Dockerfile builds only server/, so exclude everything and add that subtree back. +# An allow-list keeps the context minimal as the repo grows new top-level directories. * !server/ -# Re-exclude what lives under server/ but never belongs in an image layer: a local -# virtualenv (larger than the image itself), local secrets, and build droppings. +# Re-exclude what lives under server/ but never belongs in a layer: a local virtualenv +# (larger than the image), local secrets, and build droppings. server/.venv server/.env server/.env.* diff --git a/.env.example b/.env.example index 7434c955c..8f4ed83d5 100644 --- a/.env.example +++ b/.env.example @@ -1,9 +1,8 @@ # Copy to .env for local development. On Render these are set by render.yaml, except # OPENAI_API_KEY, which Render prompts for at deploy time. # -# Optional settings are left commented out rather than assigned an empty value. An -# empty assignment is not the same as unset: it reaches the app as '' and is taken as -# a real setting. Uncomment a line to use it. +# Optional settings stay commented out rather than assigned an empty value: '' is not the +# same as unset, and arrives at the app as a real setting. Uncomment a line to use it. # --- Required --------------------------------------------------------------------- # Graphiti calls an LLM to extract entities and facts. https://platform.openai.com/api-keys @@ -13,11 +12,10 @@ OPENAI_API_KEY= # Bearer token required by every endpoint except /healthcheck. Mandatory, with no way to switch # auth off; blank counts as missing. # -# Render generates it when the service is first created, and `docker compose` defaults it to -# insecure-local-dev-key — so leave this unset unless you want your own value, which is then what -# the local stack and the curl examples expect. Must be 16+ printable-ASCII characters: the server -# budgets failed attempts, but that can't save a key like `dev`, and header values travel as -# latin-1 with clients encoding anything outside ASCII inconsistently. +# Render generates it on first deploy and `docker compose` defaults it to insecure-local-dev-key, +# so leave this unset unless you want your own value — which the local stack and the curl examples +# then expect. Must be 16+ printable-ASCII characters: the server budgets failed attempts, but +# that can't save a key like `dev`, and headers travel as latin-1. # GRAPHITI_API_KEY= # --- Graph backend ---------------------------------------------------------------- @@ -39,11 +37,17 @@ FALKORDB_DATABASE=default_db # Bolt port; read by docker-compose.yml, which defaults it to 7687. # NEO4J_PORT= +# --- Serving ---------------------------------------------------------------------- +# The port uvicorn binds to, set for you in render.yaml. Setting it here changes nothing under +# docker compose: both API services pin it to 8000, which is what their healthchecks and +# published ports name. Edit docker-compose.yml's mapping to move where the stack is reachable. +# PORT=8000 + # --- Model ------------------------------------------------------------------------ # Any OpenAI model id; leave unset to use graphiti-core's default. MODEL_NAME=gpt-5.5 -# Concurrent LLM calls during ingestion. graphiti-core defaults to 20; this is lower to -# stay under the rate limits on a fresh OpenAI account. +# Concurrent LLM calls during ingestion. Below graphiti-core's default of 20, to stay under +# the rate limits on a fresh OpenAI account. SEMAPHORE_LIMIT=10 # Embedding model; leave unset for graphiti-core's default (text-embedding-3-small). # EMBEDDING_MODEL_NAME= diff --git a/.github/workflows/server-tests.yml b/.github/workflows/server-tests.yml index 04fae1894..5d013f4ff 100644 --- a/.github/workflows/server-tests.yml +++ b/.github/workflows/server-tests.yml @@ -1,7 +1,6 @@ name: Server Tests -# Tests for the Graphiti graph_service (REST API), in two jobs, both on PRs and pushes to -# main that touch the server: +# Tests for graph_service (the REST API), on PRs and pushes to main that touch the server: # # - unit-tests: everything needing no secrets and no database, notably the auth guards in # tests/test_auth.py. Runs on forks too, so a router included without its auth dependency @@ -24,8 +23,8 @@ on: permissions: contents: read -# Cancel superseded runs on the same ref so repeated pushes don't keep paying for -# real OpenAI calls. +# Cancel superseded runs on the same ref, so repeated pushes don't keep paying for real +# OpenAI calls. concurrency: group: server-tests-${{ github.event.pull_request.number || github.ref }} cancel-in-progress: true @@ -50,8 +49,8 @@ jobs: run: uv sync --extra dev - name: Run unit tests working-directory: server - # By marker, not filename, so a new module is picked up by default and only opts - # out by marking itself integration. --strict-markers makes a typo an error. + # By marker, not filename, so a new module is picked up by default and opts out only + # by marking itself integration. --strict-markers makes a typo an error. run: uv run pytest -m "not integration" --strict-markers -p no:cacheprovider live-server-tests: @@ -70,9 +69,8 @@ jobs: with: version: "latest" - name: Start FalkorDB - # Run the database on the host network rather than a `services:` block: on - # the depot-ubuntu runner pool bridge-network NAT silently drops packets to - # published service-container ports. Readiness is checked via `docker exec`. + # Host network rather than a `services:` block: on the depot-ubuntu runner pool, + # bridge-network NAT silently drops packets to published service-container ports. run: docker run -d --name falkordb --network host falkordb/falkordb:latest - name: Install dependencies working-directory: server @@ -93,21 +91,16 @@ jobs: DB_BACKEND: falkordb FALKORDB_HOST: localhost FALKORDB_PORT: "6379" - # gpt-5.5 is the graphiti-core default, but it requires a key/account - # with fast gpt-5.5 access; CI pins a lighter, broadly-available model so - # the live test stays fast and reliable. It exercises the same pipeline. + # graphiti-core defaults to gpt-5.5, which needs an account with fast access to it. + # A lighter, broadly-available model exercises the same pipeline. MODEL_NAME: gpt-4.1-mini - # Raise episode/LLM-call concurrency so the live episode processes quickly. + # Raised so the live episode processes quickly. SEMAPHORE_LIMIT: "20" - # pytest exits 5 when no tests are collected. The legitimate case is a fork - # PR with no OPENAI_API_KEY: the suite self-skips at module level. When the - # key IS present (same-repo run) exit 5 is unexpected — the live suite should - # have run — so fail loudly rather than report a green build that tested - # nothing. Likely causes: FalkorDB became unreachable (the suite also skips at - # module level when it can't connect, even though the prior step gated it), or - # the `integration` marker was renamed/removed so the selection matches - # nothing. Real failures (exit 1), import errors (exit 2), and usage errors - # such as a renamed test-file path (exit 4) always fail the job too. + # pytest exits 5 when nothing is collected. The legitimate case is a fork PR with no + # OPENAI_API_KEY, where the suite self-skips at module level. With the key present, exit + # 5 means the live suite should have run and didn't — FalkorDB unreachable, or the + # `integration` marker renamed — so fail rather than report a green build that tested + # nothing. Real failures (1), import errors (2), and usage errors (4) fail the job too. run: | set +e uv run pytest tests/test_live_falkordb_int.py -m integration -p no:cacheprovider diff --git a/Dockerfile b/Dockerfile index 0912da96f..e4eabd2f5 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,14 +1,11 @@ # syntax=docker/dockerfile:1.9 FROM python:3.12-slim -# The graphiti-core release installed from PyPI below, and the value of the version -# labels. This default is the single source of truth for the pin: render.yaml, -# docker-compose.yml, and local builds all inherit it, so a bump happens here and -# nowhere else. It is a PyPI release number, not this repo's own version. +# The graphiti-core PyPI release installed below, and the version labels. Single source of +# truth for the pin: render.yaml, docker-compose.yml, and local builds all inherit it. # -# 0.29.2 in particular does not work on FalkorDB: it writes episodes fine but reads them -# back empty, so /search and /episodes return nothing. Verify a write-then-search round -# trip before bumping. Release CI overrides this with the version it is publishing. +# 0.29.2 in particular is broken on FalkorDB — it writes episodes but reads them back empty, +# so /search returns nothing. Verify a write-then-search round trip before bumping. ARG GRAPHITI_VERSION=0.29.3 # Inherit build arguments for labels @@ -49,9 +46,8 @@ WORKDIR /app COPY ./server/pyproject.toml ./server/README.md ./server/uv.lock ./ COPY ./server/graph_service ./graph_service -# Install server dependencies (without graphiti-core from lockfile) -# Then install graphiti-core from PyPI at the desired version -# This prevents the stale lockfile from pinning an old graphiti-core version +# Server deps from the lockfile, then graphiti-core from PyPI, so the stale lockfile +# can't pin an old graphiti-core. ARG INSTALL_FALKORDB=false RUN --mount=type=cache,target=/root/.cache/uv \ : "${GRAPHITI_VERSION:?must be a graphiti-core release from PyPI}" && \ @@ -76,15 +72,11 @@ USER app ENV PORT=8000 EXPOSE $PORT -# Shell form, so $PORT actually reaches uvicorn: the exec form does no variable expansion, -# which is why this used to hardcode 8000 and ignore the PORT it declares above. `:-8000` -# covers PORT set but empty, which a dashboard or a .env makes easy and which is not the -# same as unset. `exec` replaces the shell, so uvicorn is PID 1 and gets SIGTERM directly -# on deploy or shutdown, rather than the shell absorbing it until the runtime kills us. +# Shell form, so $PORT reaches uvicorn at all — the exec form does no expansion. `:-8000` +# covers PORT set but empty, which a dashboard or a .env makes easy. `exec` keeps uvicorn as +# PID 1, so it gets SIGTERM directly instead of the shell absorbing it. # -# uvicorn straight from the venv, not `uv run --no-sync uvicorn`: uv is installed under -# /root/.local/bin, and /root is 0700, so the app user cannot resolve it through PATH. The -# exec form got away with it because the runtime resolves the binary as root before -# dropping privileges; a shell doing its own PATH lookup gets EACCES. PATH already puts -# /app/.venv/bin first, so this is the same interpreter uv run would have selected. +# uvicorn from the venv, not `uv run`: uv lives under /root/.local/bin and /root is 0700, so +# the app user's PATH lookup gets EACCES. Same interpreter either way — PATH puts +# /app/.venv/bin first. CMD ["sh", "-c", "exec uvicorn graph_service.main:app --host 0.0.0.0 --port ${PORT:-8000}"] diff --git a/README.md b/README.md index 059b13278..c84b6365e 100644 --- a/README.md +++ b/README.md @@ -63,6 +63,7 @@ from `graphiti-api` over Render's private network. | ----------------- | --------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | `MODEL_NAME` | `gpt-5.5` | Any OpenAI model id. | | `SEMAPHORE_LIMIT` | `10` | Concurrent LLM calls during ingestion. Deliberately below graphiti-core's default of 20, so a burst of episodes doesn't trip the rate limits on a fresh OpenAI key. Raise it once you know your account's limits. | + | `PORT` | `8000` | The port uvicorn binds to and the one Render routes to. Declared up front so the first deploy doesn't restart to rewire the network after detecting the port itself. Safe to change. | The rest is wiring you shouldn't need to touch. `DB_BACKEND` selects the FalkorDB code path; `FALKORDB_HOST` is filled in from the private service's hostname, with diff --git a/docker-compose.test.yml b/docker-compose.test.yml index c72dfb2a1..2fc1eab2a 100644 --- a/docker-compose.test.yml +++ b/docker-compose.test.yml @@ -1,7 +1,7 @@ services: graph: image: graphiti-service:${GITHUB_SHA} - # Localhost-bound, as in docker-compose.yml: the key below is committed to this repo. + # Localhost-bound, as in docker-compose.yml: the key below is committed. ports: - "127.0.0.1:8000:8000" healthcheck: diff --git a/docker-compose.yml b/docker-compose.yml index 7dc044972..de4abe811 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -3,9 +3,9 @@ services: profiles: [""] build: context: . - # Localhost-bound, not the bare "8000:8000": the key below is committed to this repo, so - # publishing on every interface would hand the graph to anyone on a shared network. Drop the - # prefix to reach this from another machine, and set your own key in the same edit. + # Localhost-bound, not the bare "8000:8000": the key below is committed, so publishing on + # every interface would hand the graph to anyone on the network. Drop the prefix to reach + # this from another machine, and set your own key in the same edit. ports: - "127.0.0.1:8000:8000" healthcheck: @@ -22,16 +22,15 @@ services: depends_on: neo4j: condition: service_healthy - # Everything else the app reads comes straight from .env, so the settings documented - # in .env.example reach the container instead of being silently dropped. Listing them - # here one by one would mean a blank value for each one the user left out, and a blank - # is not the same as unset: SEMAPHORE_LIMIT is read as int(os.getenv(...)), so '' is a - # crash on import. Absent from .env means absent from the environment. + # Everything else comes straight from .env, so the settings documented in .env.example + # reach the container. Listing them individually would pass a blank for each one left + # out, and blank is not unset: SEMAPHORE_LIMIT is read as int(os.getenv(...)), so '' is a + # crash on import. env_file: - path: .env required: false - # Compose-specific wiring, which has to win over whatever .env says: these hostnames - # only make sense on the compose network. environment takes precedence over env_file. + # Compose-specific wiring, which has to win over .env — these hostnames only exist on the + # compose network. environment takes precedence over env_file. environment: - OPENAI_API_KEY=${OPENAI_API_KEY} # Mandatory, so the stack needs one to boot. Named insecure- because it is committed and @@ -41,10 +40,9 @@ services: - NEO4J_USER=${NEO4J_USER:-neo4j} - NEO4J_PASSWORD=${NEO4J_PASSWORD:-password} - DB_BACKEND=neo4j - # Now that the Dockerfile's CMD honours PORT, pin it: the mapping above and the - # healthcheck below both name 8000 literally, so a PORT inherited from .env would - # move the server out from under them. Change the left-hand side of the mapping to - # publish somewhere else. + # Pinned because the CMD honours PORT: the mapping and healthcheck both name 8000, so a + # PORT inherited from .env would move the server out from under them. To publish + # elsewhere, change the left-hand side of the mapping. - PORT=8000 neo4j: image: neo4j:5.26.2 @@ -59,7 +57,7 @@ services: timeout: 10s retries: 10 start_period: 3s - # Localhost-bound, as above, with more at stake: NEO4J_PASSWORD defaults to `password` and + # Localhost-bound, as above, with more at stake: the password defaults to `password` and # Bolt is unmediated write access. ports: - "127.0.0.1:7474:7474" # HTTP @@ -73,16 +71,16 @@ services: # Kept in step with render.yaml so local dev matches what Render runs. image: falkordb/falkordb:v4.20.1 profiles: ["falkordb"] - # Localhost-bound, as above. FalkorDB has no password at all; on Render it is a pserv - # reachable only from graphiti-api, and this is the closest local equivalent. + # Localhost-bound, as above, and FalkorDB has no password at all. On Render it's a pserv + # reachable only from graphiti-api; this is the closest local equivalent. ports: - "127.0.0.1:6379:6379" volumes: - falkordb_data:/var/lib/falkordb/data environment: - # REDIS_ARGS goes to redis-server; FALKORDB_ARGS goes to the module. The flags that - # used to sit in FALKORDB_ARGS were redis-server flags in the module's slot, and - # both were already the defaults. Append-only matches what render.yaml runs. + # REDIS_ARGS goes to redis-server, FALKORDB_ARGS to the module. The flags that used to + # sit in FALKORDB_ARGS were redis-server flags in the module's slot, and both were + # already the defaults. Append-only matches render.yaml. - REDIS_ARGS=--appendonly yes healthcheck: test: ["CMD", "redis-cli", "-p", "6379", "ping"] @@ -119,8 +117,8 @@ services: - FALKORDB_PORT=6379 - FALKORDB_DATABASE=default_db - DB_BACKEND=falkordb - # Pinned for the same reason as the graph service above. Published on 8001 to leave - # 8000 free for the neo4j profile; the container still listens on 8000. + # Pinned as on the graph service above. Published on 8001 to leave 8000 free for the + # neo4j profile; the container still listens on 8000. - PORT=8000 volumes: diff --git a/examples/render/demo.sh b/examples/render/demo.sh index 2ead7a860..a1fb9a959 100755 --- a/examples/render/demo.sh +++ b/examples/render/demo.sh @@ -13,25 +13,17 @@ # ask "who owns X?" the current answer, one line # timeline "leads the" every version of that fact, oldest first # -# There is deliberately no clear helper. DELETE /group and POST /clear return -# {"success": true} on this deployment and delete nothing — they query the default -# graph while the data lives in a graph named after the group_id. To actually erase a -# group, open a shell on graphiti-falkordb and run `redis-cli GRAPH.DELETE `; -# to start clean without deleting anything, just `use_group` a new name. +# These wrap POST /search and /messages and project the response down to the interesting +# fields; see $GRAPHITI_URL/docs for the full shape. # -# Every command is one word with no quoting or line continuations, because a -# multi-line curl that loses its -H flag mid-paste sends the body without a JSON -# content type and the API replies 422 about a string it can't read as an object. -# -# These wrap POST /search and /messages and project the response down to the -# interesting fields. The raw endpoints return uuid/created_at/expired_at too — -# see $GRAPHITI_URL/docs for the full shape. +# There is deliberately no clear helper: DELETE /group and POST /clear return +# {"success": true} and delete nothing, because they query the default graph while the data +# lives in a graph named after the group_id. To actually erase a group, run +# `redis-cli GRAPH.DELETE ` on graphiti-falkordb; to start clean, use_group a new name. -# Executing this file would define the functions in a subshell that exits -# immediately, so catch that and say so rather than appearing to do nothing. -# zsh needs its own test: ZSH_VERSION being set says nothing about how the file -# was loaded, so `zsh demo.sh` would slip past a bash-only check. ZSH_EVAL_CONTEXT -# gains a :file component when sourced and stays at toplevel when executed. +# Executing this file would define the functions in a subshell that exits immediately, so say +# so rather than appear to do nothing. zsh needs its own test: ZSH_VERSION says nothing about +# how the file was loaded, but ZSH_EVAL_CONTEXT gains a :file component when sourced. _graphiti_sourced=0 if [ -n "$ZSH_VERSION" ]; then case "$ZSH_EVAL_CONTEXT" in *:file*) _graphiti_sourced=1 ;; esac @@ -55,18 +47,18 @@ unset _graphiti_sourced command -v jq >/dev/null || echo 'demo.sh: jq not found — brew install jq' -# curl with the bearer header attached, built in one place so a new helper cannot forget it. -# Only health() bypasses it. Read per call, so re-exporting a rotated key needs no re-source. +# curl with the bearer header attached, in one place so a new helper can't forget it. Only +# health() bypasses it. Read per call, so a rotated key needs no re-source. # -# -H @- takes the header from stdin, keeping the key out of argv where `ps` would expose it to -# other users. Needs curl 7.55+ (2017). No helper sends a body on stdin, so stdin is free. +# -H @- takes the header from stdin, keeping the key out of argv where `ps` would expose it. +# Needs curl 7.55+ (2017); no helper sends a body on stdin, so stdin is free. _graphiti_curl() { printf 'Authorization: Bearer %s\n' "$GRAPHITI_API_KEY" | curl -H @- "$@" } -# $1 is an HTTP status. Returns 0 on 2xx, else explains on stderr and returns 1, so a caller -# can end with it or use `|| return 1` mid-function. Keyed off the status, not the body: the -# 401 wording is auth.py's to change, and matching it would silently misreport. +# $1 is an HTTP status. Returns 0 on 2xx, else explains on stderr and returns 1, so a caller can +# end with it or use `|| return 1`. Keyed off the status, not the body, whose wording is auth.py's +# to change. _graphiti_check_status() { case "$1" in 2*) return 0 ;; @@ -74,8 +66,8 @@ _graphiti_check_status() { echo ' 401 — GRAPHITI_API_KEY was rejected. Re-copy it from the Render Dashboard:' >&2 echo ' graphiti-api -> Environment -> GRAPHITI_API_KEY' >&2 ;; 429) - # Only reachable with a wrong key: the service never limits one carrying the right - # key. So say what a bare "HTTP 429" would not — this is still the key. + # Only reachable with a wrong key, since the service never limits a right one — so say + # what a bare "HTTP 429" would not. echo ' 429 — too many rejected keys; the service is throttling them. The key is' >&2 echo ' still wrong. Re-copy it, then wait a minute and retry:' >&2 echo ' graphiti-api -> Environment -> GRAPHITI_API_KEY' >&2 ;; @@ -87,20 +79,18 @@ _graphiti_check_status() { return 1 } -# Where the sample episode lives, resolved once at source time so the helpers -# work from any directory afterwards. +# Resolved once at source time, so the helpers work from any directory afterwards. GRAPHITI_EPISODE="${GRAPHITI_EPISODE:-$(cd "$(dirname "${BASH_SOURCE[0]:-$0}")" && pwd)/sample-episode.json}" -# Point every helper at a group. One group per take: re-ingesting into a group -# that already holds facts dedupes into them, so nothing visibly happens. -# Named use_group rather than take because oh-my-zsh already defines take. +# Point every helper at a group. One group per take: re-ingesting into a group that already holds +# facts dedupes into them, so nothing visibly happens. Not named `take` — oh-my-zsh defines that. use_group() { if [ -z "$1" ]; then echo ' usage: use_group take2' >&2 return 1 fi - # The name goes into the DELETE /group/{id} path unencoded, so a slash would - # silently address a different route and a space would break the URL outright. + # The name goes into the DELETE /group/{id} path unencoded, so a slash would address a + # different route and a space would break the URL outright. case "$1" in *[!A-Za-z0-9._-]*) echo " '$1' won't work as a group name — use letters, digits, . _ or - only" >&2 @@ -117,20 +107,19 @@ health() { metrics=$(curl -sS -o /dev/null -w '%{http_code} %{time_total}' "$GRAPHITI_URL/healthcheck") http_code="${metrics% *}" printf ' healthcheck → HTTP %s in %ss\n' "$http_code" "${metrics#* }" - # Last, so its status becomes health's: `health && ingest` used to ingest against a - # service that never answered. + # Last, so its status becomes health's: `health && ingest` used to run against a service + # that never answered. _graphiti_check_status "$http_code" } -# Rewrite the sample episode's group_id to match $GRAPHITI_GROUP and POST it. -# --data-binary rather than -d: -d strips newlines, which is harmless for JSON -# but makes the payload unreadable if you ever need to echo it back. +# Rewrite the sample episode's group_id to match $GRAPHITI_GROUP and POST it. --data-binary +# rather than -d, which strips newlines and leaves the payload unreadable if echoed back. ingest() { # http_code, not status: status is read-only in zsh, which most readers are sourcing into. local file="${1:-$GRAPHITI_EPISODE}" tmp body metrics http_code [ -f "$file" ] || { echo " no episode file at $file" >&2; return 1; } - # An explicit XXXXXX template: macOS accepts a bare prefix after -t, GNU and - # BusyBox mktemp reject it, so -t alone would break the helper on Linux. + # An explicit XXXXXX template: macOS accepts a bare prefix after -t, GNU and BusyBox + # mktemp reject it. tmp=$(mktemp "${TMPDIR:-/tmp}/graphiti-episode.XXXXXX") || return 1 body=$(mktemp "${TMPDIR:-/tmp}/graphiti-ingest.XXXXXX") || { rm -f "$tmp"; return 1; } jq --arg g "$GRAPHITI_GROUP" '.group_id = $g' "$file" > "$tmp" || { rm -f "$tmp" "$body"; return 1; } @@ -140,21 +129,21 @@ ingest() { --data-binary @"$tmp") http_code="${metrics% *}" printf ' ingest → HTTP %s in %ss (group %s)\n' "$http_code" "${metrics#* }" "$GRAPHITI_GROUP" - # A 202 body is just an ack, so only surface it on an error — and not on a 401, where the + # A 202 body is just an ack, so surface it only on an error — and not on a 401, where the # hint below says the same thing with the fix attached. case "$http_code" in 2*|401) ;; *) jq -c . "$body" 2>/dev/null || cat "$body" ;; esac rm -f "$tmp" "$body" - # Last, so its status becomes ingest's: a non-2xx is a failure, not a line that scrolls - # past looking like progress. + # Last, so its status becomes ingest's: a non-2xx is a failure, not a line that scrolls past + # looking like progress. _graphiti_check_status "$http_code" } -# POST /search and emit the body, then the HTTP status on a final line. $1 query, $2 -# max_facts (default 10). Only _graphiti_facts calls this, and it splits the two apart: the -# status is what tells a rejected key from a service that is simply down. +# POST /search and emit the body, then the HTTP status on a final line. $1 query, $2 max_facts +# (default 10). Only _graphiti_facts calls this, and splits the two apart: the status is what +# tells a rejected key from a service that is simply down. _graphiti_search() { _graphiti_curl -s -w '\n%{http_code}' -X POST "$GRAPHITI_URL/search" \ -H 'Content-Type: application/json' \ @@ -162,9 +151,9 @@ _graphiti_search() { '{group_ids: [$g], query: $q, max_facts: $n}')" } -# Emit the search response only if it is JSON with a facts array, so callers can -# tell "the service is down" from "the graph knows nothing" and say which. -# $1 query, $2 max_facts. Diagnoses its own failure on stderr, so callers just propagate. +# Emit the search response only if it is JSON with a facts array, so callers can tell "the service +# is down" from "the graph knows nothing". $1 query, $2 max_facts. Diagnoses its own failure on +# stderr, so callers just propagate. _graphiti_facts() { local response json http_code detail response=$(_graphiti_search "$1" "${2:-10}") @@ -177,8 +166,8 @@ _graphiti_facts() { fi # A 401 gets the fix rather than its body, which says the same without one. [ "$http_code" = 401 ] && { _graphiti_check_status "$http_code"; return 1; } - # FastAPI reports other errors as {"detail": ...}, so a detail field means the service - # answered and rejected us — quoting it beats guessing it is down. + # FastAPI reports other errors as {"detail": ...}, so a detail field means the service answered + # and rejected us — quoting it beats guessing it is down. detail=$(printf '%s' "$json" | jq -r 'if (.detail|type) == "string" then .detail else empty end' 2>/dev/null) if [ -n "$detail" ]; then echo " $GRAPHITI_URL/search refused the request: $detail" >&2 @@ -189,23 +178,19 @@ _graphiti_facts() { return 1 } -# How many facts the current group returns for $1, or empty if the service did not answer -# with usable JSON. Callers must handle empty: a cold start or a 502 yields no output, and -# feeding that to [ ] produces "integer expression expected" instead of anything a reader -# can act on. +# How many facts the current group returns for $1, or empty if the service did not answer with +# usable JSON. Callers must handle empty: feeding it to [ ] gives "integer expression expected". # -# The quiet variant of _graphiti_facts: watch_ingest's loop carries the last count forward on -# a dropped request, and a diagnostic per blip would scroll past. Its baseline call has -# already made any noise. +# The quiet variant of _graphiti_facts, because watch_ingest's loop carries the last count forward +# on a dropped request and a diagnostic per blip would scroll past. _graphiti_count() { _graphiti_facts "$1" 50 2>/dev/null | jq -e '.facts | length' 2>/dev/null } -# The current answer, on one line, rendered as the graph edge it came from so it -# reads the same as timeline output. /search returns facts in relevance order, so -# narrow to the newest valid_at first and then keep the most relevant of those: -# sorting on valid_at alone tie-breaks arbitrarily between same-day facts and can -# answer a different question than the one asked. +# The current answer, on one line, rendered as the graph edge it came from so it reads like +# timeline output. /search returns facts in relevance order, so narrow to the newest valid_at and +# then keep the most relevant of those: sorting on valid_at alone tie-breaks arbitrarily between +# same-day facts and can answer a different question than the one asked. ask() { local out json if [ -z "$1" ]; then @@ -226,16 +211,13 @@ ask() { printf '%s\n' "$out" } -# Every version of a fact, oldest first, as the graph edge behind it: the -# relationship label and the window the edge is valid for. $1 is a regex matched -# against fact text. unique_by guards against the same episode having been -# ingested twice. Named timeline, not history, so it doesn't shadow the builtin. +# Every version of a fact, oldest first, as the graph edge behind it: the relationship label and +# the window it is valid for. $1 is a regex matched against fact text; unique_by guards against +# the same episode being ingested twice. Not named history, which would shadow the builtin. # -# This stops short of drawing (Alex) ──LEADS──▶ (payments team) because /search -# returns no node names: FactResult has the edge label and text but not the -# edge's two endpoints, and the API exposes no node getter to resolve them. For -# real endpoints, add source/target to FactResult in -# server/graph_service/dto/retrieve.py and resolve the names in +# Stops short of drawing (Alex) ──LEADS──▶ (payments team) because /search returns no node names: +# FactResult carries the edge label and text but not its endpoints. To get real ones, add +# source/target to FactResult in server/graph_service/dto/retrieve.py and resolve the names in # get_fact_result_from_edge. timeline() { local out json @@ -250,15 +232,13 @@ timeline() { | if length == 0 then [" (no facts match \($pat))"] | .[] else .[] | (if .valid_at then .valid_at[0:10] else "undated " end) as $from - # Only claim an end date when the edge actually carries one. Printing - # "→ now" for a null invalid_at asserts the superseded fact is still - # live, which contradicts the fact that replaced it. + # Only claim an end date when the edge carries one: printing "→ now" for a null + # invalid_at would assert a superseded fact is still live. | (if .invalid_at then " → \(.invalid_at[0:10])" else "" end) as $to | " \($from)\($to) ──\(.name)──▶ \(.fact)" end' 2>/dev/null) - # The service answered, so empty output here means jq itself failed — and the - # only way that happens is an invalid regex in $1, since a zero-match search - # takes the length == 0 branch above and prints its own line. + # The service answered, so empty output means jq failed — and the only way that happens is an + # invalid regex in $1, since a zero-match search takes the length == 0 branch above. if [ -z "$out" ]; then echo " '$1' is not a valid regex — try plain words:" >&2 echo ' timeline "leads the payments"' >&2 @@ -267,15 +247,12 @@ timeline() { printf '%s\n' "$out" } -# Poll until the fact count stops changing. Prints only when it moves, so the -# output stays short. Ingestion is serial and LLM-bound, so expect ~30s for the -# three-message sample. +# Poll until the fact count stops changing, printing only when it moves. Ingestion is serial and +# LLM-bound, so expect ~30s for the three-message sample. # -# The count has to rise above whatever the group already held, not just above -# zero: re-ingesting into a group that still has the previous run's facts -# dedupes into them, so the count can sit flat at its starting value and the -# watcher would otherwise declare victory on the *old* data. Start each take in -# an empty group. +# The count has to rise above what the group already held, not just above zero: a re-ingest dedupes +# into the previous run's facts, so the count can sit flat and the watcher would declare victory on +# the *old* data. Start each take in an empty group. watch_ingest() { local force='' # Consume --force before reading the query, or it would become the query. @@ -284,13 +261,12 @@ watch_ingest() { local t0 last count stable=0 elapsed baseline json t0=$(date +%s) - # Via _graphiti_facts, not the silent _graphiti_count: without a baseline there is no - # telling new facts from old, so this is the point to stop and say why — a rejected key - # surfaces here rather than as three minutes of polling that never moves. + # Via _graphiti_facts, not the silent _graphiti_count: without a baseline there is no telling + # new facts from old, so a rejected key surfaces here rather than as three minutes of polling. json=$(_graphiti_facts "$query" 50) || return 1 baseline=$(printf '%s' "$json" | jq '.facts | length') - # Refuse rather than poll for three minutes and time out: on a populated group - # the count legitimately may never move, so there is nothing to wait for. + # Refuse rather than poll for three minutes and time out: on a populated group the count may + # legitimately never move, so there is nothing to wait for. if [ "$baseline" -gt 0 ] && [ -z "$force" ]; then echo " group '$GRAPHITI_GROUP' already holds $baseline facts — nothing to watch," >&2 echo " since a re-ingest dedupes into them and the count never moves." >&2 @@ -303,8 +279,8 @@ watch_ingest() { last=-1 while true; do count=$(_graphiti_count "$query") - # A dropped poll is not a change in the graph. Carry the last known count - # forward, or a blip would reset the stability counter and stall the watcher. + # A dropped poll is not a change in the graph: carry the last count forward, or a blip + # resets the stability counter and stalls the watcher. [ -n "$count" ] || count=$last elapsed=$(( $(date +%s) - t0 )) if [ "$count" != "$last" ]; then @@ -314,14 +290,12 @@ watch_ingest() { else stable=$(( stable + 1 )) fi - # Episodes are processed one at a time, and the gap between one finishing and - # the next producing facts runs to ~22s, so a short plateau is not the end of - # the queue — it is the middle of it. 8 polls x 4s = 32s of no movement gives - # enough margin; anything less declares victory on a half-built graph and the - # answers come out of date. + # Episodes are processed one at a time and the gap between one finishing and the next + # producing facts runs to ~22s, so a short plateau is the middle of the queue, not the end + # of it. 8 polls x 4s = 32s of no movement gives enough margin; less declares victory on a + # half-built graph. [ "$stable" -ge 8 ] && [ "$count" -gt "$baseline" ] && break - # Timing out is a failure, not a finish — don't follow it with "done", which - # would read as a successful drain in the recording. + # Timing out is a failure, not a finish — don't follow it with "done". if [ "$elapsed" -gt 180 ]; then printf ' gave up after 180s at %s facts (started from %s)\n' "$count" "$baseline" >&2 return 1 diff --git a/render.yaml b/render.yaml index 794eb42d3..76c5ec1c1 100644 --- a/render.yaml +++ b/render.yaml @@ -3,11 +3,12 @@ # graphiti-api public REST service, built from the repo's root Dockerfile # graphiti-falkordb the graph store, private: reachable only from graphiti-api # -# Secrets are never written here. OPENAI_API_KEY is declared sync: false, so Render -# prompts for it at deploy time and stores it outside the repo. +# No secrets here: OPENAI_API_KEY is sync: false, so Render prompts at deploy time. previews: - generation: off + # Quoted, because bare off is a YAML boolean and the schema types this field as a string + # enum — unquoted it fails `render blueprints validate`. + generation: 'off' projects: - name: graphiti @@ -18,12 +19,10 @@ projects: name: graphiti-falkordb runtime: image region: oregon - # FalkorDB keeps the whole graph in RAM and only writes the append-only file - # to disk, so this plan is the real ceiling on graph size. Standard (2 GB) is - # the smallest that leaves room to grow; the disk below just has to outlast it. + # FalkorDB keeps the whole graph in RAM, so the plan is the real ceiling on + # graph size. Standard (2 GB) is the smallest with room to grow. plan: standard - # Pinned, not :latest — every fork deploys this exact version, so an upstream - # bump can't break new deploys out from under you. Bump it deliberately. + # Pinned, not :latest, so an upstream bump can't break new deploys. image: url: docker.io/falkordb/falkordb:v4.20.1 disk: @@ -34,9 +33,8 @@ projects: # Append-only file, so the graph survives a restart or redeploy. - key: REDIS_ARGS value: --appendonly yes - # The image otherwise also serves the FalkorDB Browser UI on :3000. Nothing - # uses it here, and leaving it on gives Render's port scanner a second port - # to choose from. Off, so 6379 is the only thing listening. + # The image also serves the FalkorDB Browser on :3000. Unused here, and off + # leaves 6379 as the only port Render's scanner can pick. - key: BROWSER value: '0' @@ -44,46 +42,40 @@ projects: name: graphiti-api runtime: docker region: oregon - # The API idles near 100 MB and spends most of each request waiting on OpenAI, - # so it needs far less than the graph store does. + # The API idles near 100 MB and spends most of each request waiting on OpenAI. plan: starter dockerfilePath: ./Dockerfile dockerContext: . healthCheckPath: /healthcheck envVars: - # Declared on the service, not in an env var group: sync: false is silently - # ignored inside a group, so Render would never prompt for the value. + # On the service, not in an env var group: sync: false is silently ignored + # inside a group, so Render would never prompt for the value. - key: OPENAI_API_KEY sync: false - # Bearer token for every endpoint except /healthcheck, and the server will not - # start without it, so a fork is closed from the moment it goes live. Read the - # value from the Dashboard (Environment tab) after the first deploy. + # Bearer token for every endpoint except /healthcheck; the server won't start + # without it, so a fork is closed from the moment it goes live. Read the value + # from the Dashboard (Environment tab) after the first deploy. # - # generateValue rather than sync: false keeps the one-click path one click. - # Generated once and persistent across syncs, so rotation is a manual edit here or - # in the Dashboard; a replacement must be 16+ printable-ASCII characters or the - # service refuses to start — see graph_service/config.py. + # generateValue rather than sync: false keeps the one-click path one click. It + # persists across syncs, so rotation is a manual edit here or in the Dashboard; + # a replacement must be 16+ printable-ASCII characters (see config.py). - key: GRAPHITI_API_KEY generateValue: true - # The port uvicorn binds to, and the one Render routes to. Declared rather - # than left to Render's detection: without it Render starts out expecting its - # default 10000, discovers the real port, and restarts the deploy to rewire - # the network — a slower first deploy and a "Restarting deploy" line that - # reads like a failure. Safe to change, as the Dockerfile's CMD passes this - # through to uvicorn; nothing else has to be edited to match. + # The port uvicorn binds to and the one Render routes to. Declared so the first + # deploy doesn't start on Render's default 10000, detect the real port, and + # restart to rewire the network. Safe to change: the CMD passes it through. - key: PORT value: '8000' - # Render exposes env vars to the Docker build as build args, which is how - # the root Dockerfile picks up the graphiti-core FalkorDB extra. + # Render passes env vars to the Docker build as build args, which is how the + # root Dockerfile picks up the graphiti-core FalkorDB extra. - key: INSTALL_FALKORDB value: 'true' - # The graphiti-core version is deliberately not set here. It is pinned once, - # as the GRAPHITI_VERSION build arg default in the Dockerfile, so a bump is - # a one-line change rather than three configs to keep in step. Setting it - # here would override that pin for this service only. + # GRAPHITI_VERSION is deliberately unset: it's pinned once as the Dockerfile's + # build-arg default, so a bump is one line. Setting it here overrides that pin + # for this service only. - key: DB_BACKEND value: falkordb @@ -100,8 +92,7 @@ projects: # The model Graphiti uses to extract entities and facts. Any OpenAI model id. - key: MODEL_NAME value: gpt-5.5 - # Concurrent LLM calls during ingestion. Held below graphiti-core's default - # of 20 so a burst of episodes doesn't trip OpenAI rate limits on a new key. - # Raise it once you know your account's limits. + # Concurrent LLM calls during ingestion. Below graphiti-core's default of 20 so + # a burst of episodes doesn't trip rate limits on a new OpenAI key. - key: SEMAPHORE_LIMIT value: '10' diff --git a/server/graph_service/auth.py b/server/graph_service/auth.py index 54e288c4d..2f90d7c8a 100644 --- a/server/graph_service/auth.py +++ b/server/graph_service/auth.py @@ -1,8 +1,8 @@ """Bearer-token auth for the graph endpoints. Every endpoint except /healthcheck requires GRAPHITI_API_KEY, with no way to switch it off: the -API writes to a shared graph and spends the deployment's OpenAI key on every episode. config.py -requires the key at startup, so this module can assume there is one. +API writes to a shared graph and spends the deployment's OpenAI key. config.py requires the key +at startup, so this module can assume there is one. """ import hashlib @@ -16,22 +16,22 @@ from graph_service.config import ZepEnvDep -# auto_error=False so a missing or non-Bearer header arrives as None: HTTPBearer answers 403 on -# its own, and the right answer is a 401 naming the scheme to retry with. +# auto_error=False so a missing or non-Bearer header arrives as None: HTTPBearer would answer 403 +# on its own, and the right answer is a 401 naming the scheme to retry with. _bearer = HTTPBearer(auto_error=False, description='The GRAPHITI_API_KEY set on this service.') # Rejections allowed per window before answering 429, so the key can't be worked through. # # Global, not per-IP: behind Render's proxy every request carries the proxy's address, and -# trusting X-Forwarded-For would hand an attacker the bucket key. It can't lock out a real client +# trusting X-Forwarded-For would hand an attacker the bucket key. It can't lock out a real client, # because require_api_key checks the key before the budget. In-process, so N instances allow N # times this — a shared counter would put FalkorDB on the auth path. MAX_FAILED_AUTH = 10 FAILED_AUTH_WINDOW_SECONDS = 60 -# The last MAX_FAILED_AUTH rejection times, oldest first. maxlen does the eviction, so the budget -# is spent exactly when the deque is full and its oldest entry is still in the window. monotonic, -# so an NTP correction can't resize the window. No lock: single event loop. +# The last MAX_FAILED_AUTH rejection times, oldest first, so the budget is spent exactly when the +# deque is full and its oldest entry is still in the window. monotonic, so an NTP correction can't +# resize the window. No lock: single event loop. _recent_rejections: deque[float] = deque(maxlen=MAX_FAILED_AUTH) diff --git a/server/graph_service/config.py b/server/graph_service/config.py index c5815f13d..0d77fd27f 100644 --- a/server/graph_service/config.py +++ b/server/graph_service/config.py @@ -18,30 +18,29 @@ def _strip(value: Any) -> Any: return value.strip() if isinstance(value, str) else value -# A .env file and a dashboard UI both make it easy to define a variable with an empty -# value, which is not the same thing as leaving it out: '' arrives here as a real setting -# and gets passed on to the clients. A blank model_name would be sent to OpenAI as the -# model to use, and a blank falkordb_port would fail validation before the app can boot. +# A .env file and a dashboard UI both make an empty value easy, which is not the same as +# leaving it out: '' arrives as a real setting and is passed on to the clients. A blank +# model_name would be sent to OpenAI as the model to use. # -# One case this cannot reach: the OpenAI SDK falls back to reading OPENAI_BASE_URL from -# the environment when it is handed base_url=None, so a blank OPENAI_BASE_URL still ends -# up as '' inside the client, and requests go out with no host. Leave it unset, not empty. +# One case this can't reach: the OpenAI SDK falls back to reading OPENAI_BASE_URL from the +# environment when handed base_url=None, so a blank one still ends up as '' inside the client +# and requests go out with no host. Leave it unset, not empty. OptionalStr = Annotated[str | None, BeforeValidator(_blank_to_none)] OptionalInt = Annotated[int | None, BeforeValidator(_blank_to_none)] -# The same idea for a setting that has to be present: strip first, so whitespace fails min_length +# Same idea for a setting that has to be present: strip first, so whitespace fails min_length # rather than passing as a one-space secret, and a key pasted with a trailing newline survives. RequiredStr = Annotated[str, BeforeValidator(_strip)] -# Entropy floor for graphiti_api_key. auth.py's budget stops a stranger working through the -# keyspace; this stops a key already at the front of it. Named so tests/test_auth.py can pin the -# boundary against it rather than restating the number. +# Entropy floor for graphiti_api_key: auth.py's budget stops a stranger working through the +# keyspace, this stops a key already at the front of it. Named so tests/test_auth.py can pin the +# boundary without restating the number. MIN_API_KEY_LENGTH = 16 class Settings(BaseSettings): - # Rejected when blank rather than defaulted, so a missing key fails the deploy at - # startup instead of turning every background ingestion into a 401 nobody is watching. + # Rejected when blank rather than defaulted, so a missing key fails the deploy instead of + # turning every background ingestion into a 401 nobody is watching. openai_api_key: RequiredStr = Field(min_length=1) openai_base_url: OptionalStr = None model_name: OptionalStr = None @@ -54,17 +53,17 @@ class Settings(BaseSettings): falkordb_username: OptionalStr = None falkordb_password: OptionalStr = None falkordb_database: OptionalStr = None - # Only these two backends are wired up in zep_graphiti, so a typo should be a startup - # error naming the valid values, not a silent fall-through to the Neo4j branch. + # Only these two are wired up in zep_graphiti, so a typo should be a startup error naming the + # valid values, not a silent fall-through to the Neo4j branch. db_backend: Literal['neo4j', 'falkordb'] = 'neo4j' # Bearer token for every endpoint except /healthcheck, mandatory: this API writes to a shared - # graph and spends openai_api_key on every episode. Missing fails startup, because an open - # service and one that 401s everything both look healthy to Render. + # graph and spends openai_api_key. Missing fails startup, because an open service and one that + # 401s everything both look healthy to Render. # # Printable ASCII is what survives an HTTP header: values travel as latin-1 and clients - # disagree about encoding anything outside it, so a key with an accent authenticates for some - # and not others. Enforced here rather than in auth.py so a bad rotation fails the deploy - # naming the problem, instead of 401ing the operator who set it. + # disagree about anything outside it, so an accented key authenticates for some clients and + # not others. Enforced here, not in auth.py, so a bad rotation fails the deploy naming the + # problem instead of 401ing the operator who set it. # # Caveat: pydantic echoes the offending value into the deploy log. Only ever a key that never # authenticated, and scrubbing it means catching ValidationError and degrading every other diff --git a/server/graph_service/main.py b/server/graph_service/main.py index 01db9022d..0041eb7b6 100644 --- a/server/graph_service/main.py +++ b/server/graph_service/main.py @@ -17,10 +17,10 @@ async def lifespan(_: FastAPI): await shutdown_graphiti() -# /docs, /redoc and /openapi.json stay public, departing from the usual advice to gate them: this -# template's source is public, so the schema is the same map routers/ are, and a browser can't put -# an Authorization header on a plain navigation. Swagger's Authorize button picks the scheme up -# from the dependency below. Revisit if this ever ships as a private service. +# /docs, /redoc and /openapi.json stay public, against the usual advice: this template's source is +# public, so the schema maps nothing routers/ doesn't, and a browser can't put an Authorization +# header on a plain navigation. Swagger's Authorize button picks the scheme up from the dependency +# below. Revisit if this ever ships as a private service. app = FastAPI(title='Graphiti API', lifespan=lifespan) diff --git a/server/graph_service/zep_graphiti.py b/server/graph_service/zep_graphiti.py index d21f5d3c1..274a00130 100644 --- a/server/graph_service/zep_graphiti.py +++ b/server/graph_service/zep_graphiti.py @@ -85,17 +85,16 @@ def _create_openai_clients( ) -> tuple[OpenAIClient, OpenAIEmbedder, OpenAIRerankerClient]: """Build the OpenAI-backed clients Graphiti needs, configured from settings. - These have to be constructed with the settings rather than patched afterwards: each - client builds its AsyncOpenAI in __init__ out of config.api_key and config.base_url, - so assigning to the config later leaves the already-built client pointing elsewhere. + Constructed with the settings rather than patched afterwards: each client builds its + AsyncOpenAI in __init__ from config.api_key and config.base_url, so assigning to the + config later leaves the built client pointing elsewhere. """ api_key = settings.openai_api_key base_url = settings.openai_base_url embedder_config = OpenAIEmbedderConfig(api_key=api_key, base_url=base_url) - # Assigned only when set, rather than passed to the constructor: embedding_model is - # typed str (defaulting to a real model name), so passing None would fail validation. - # LLMConfig.model below is str | None, which is why it can be passed straight in. + # Assigned only when set, rather than passed to the constructor: embedding_model is typed + # str, so None fails validation. LLMConfig.model below is str | None, so it goes straight in. if settings.embedding_model_name is not None: embedder_config.embedding_model = settings.embedding_model_name @@ -104,8 +103,8 @@ def _create_openai_clients( config=LLMConfig(api_key=api_key, base_url=base_url, model=settings.model_name) ), OpenAIEmbedder(config=embedder_config), - # No model override here. The reranker scores passages off logprobs on a one-token - # completion, which is a different job from extraction — leave it on its default. + # No model override: the reranker scores passages off logprobs on a one-token + # completion, a different job from extraction. Leave it on its default. OpenAIRerankerClient(config=LLMConfig(api_key=api_key, base_url=base_url)), ) @@ -131,7 +130,6 @@ def _create_graphiti_client(settings: Settings) -> ZepGraphiti: cross_encoder=cross_encoder, ) else: - # Validate Neo4j settings are present if not all([settings.neo4j_uri, settings.neo4j_user, settings.neo4j_password]): raise ValueError( 'Neo4j configuration (neo4j_uri, neo4j_user, neo4j_password) is required ' @@ -149,17 +147,14 @@ def _create_graphiti_client(settings: Settings) -> ZepGraphiti: # One client for the life of the process, rather than one per request. # -# Both graph drivers fire off build_indices_and_constraints() as an unawaited background task -# from their constructor. Building a client per request therefore started an index build per -# request, and closing that client when the request ended cut the task's connection mid-query -# — which is where the 'Connection closed by server' tracebacks came from. The same close also -# broke /messages, which hands its ingestion work to the async worker: those jobs outlive the -# request, so they ran against a client that had already been closed and only survived on the -# redis client's transparent reconnect. +# Both graph drivers fire off build_indices_and_constraints() as an unawaited background task from +# their constructor, so a client per request meant an index build per request — and closing that +# client at the end of the request cut the task's connection mid-query, which is where the +# 'Connection closed by server' tracebacks came from. It also broke /messages, whose ingestion +# outlives the request and only survived on the redis client's transparent reconnect. # -# Sharing one client fixes both: the index build happens once, awaited, at startup, and queued -# jobs keep a client that stays open. It also stops rebuilding the OpenAI clients and the -# connection pool on every request. +# Sharing one client fixes both: the index build happens once, awaited, at startup, and queued jobs +# keep a client that stays open. It also stops rebuilding the OpenAI clients per request. _graphiti_client: ZepGraphiti | None = None diff --git a/server/tests/test_auth.py b/server/tests/test_auth.py index 752401298..2d5570bfc 100644 --- a/server/tests/test_auth.py +++ b/server/tests/test_auth.py @@ -1,9 +1,9 @@ """Auth tests for the graph endpoints. Need no OpenAI key and no database, so they run in the default `make test`. Nothing asserts a -successful response: the app is built without its lifespan, so anything past auth reaches a -handler with no Graphiti client and fails. Only whether require_api_key let the request through -is asserted — see _assert_passed_auth. +successful response: the app is built without its lifespan, so anything past auth reaches a handler +with no Graphiti client and fails. These assert only whether require_api_key let the request +through — see _assert_passed_auth. The real graph_service.main is reloaded per case rather than a stand-in assembled, because its wiring is what these guard: the likeliest regression is a router included without the auth @@ -25,9 +25,9 @@ SECRET = 'test-api-key-6Yp2Qk' assert len(SECRET) >= MIN_API_KEY_LENGTH, 'the fixture key must itself be a valid key' -# The allowlist the app is held to; every other route must carry the auth dependency. An -# allowlist, not protected prefixes, so an unpredicted route name fails by default. The docs are -# deliberately public — see main.py. +# The allowlist the app is held to; every other route must carry the auth dependency. An allowlist, +# not protected prefixes, so an unpredicted route name fails by default. The docs are deliberately +# public — see main.py. PUBLIC_PATHS = {'/healthcheck', '/docs', '/docs/oauth2-redirect', '/redoc', '/openapi.json'} DOCS_PATHS = ['/openapi.json', '/docs', '/redoc'] @@ -37,9 +37,8 @@ def _fresh_rejection_budget(): """Empty the failed-auth budget around every case. Autouse, not optional: the budget lives in graph_service.auth, which build_app does not - reload, so rejections would accumulate module-wide. This file provokes more than - MAX_FAILED_AUTH of them, so cases asserting 401 would see 429 — order-dependently, which a - single -k run would hide. + reload, so rejections accumulate module-wide. This file provokes more than MAX_FAILED_AUTH of + them, so cases asserting 401 would see 429 — order-dependently, which a single -k run hides. """ auth._recent_rejections.clear() yield @@ -50,20 +49,18 @@ def _fresh_rejection_budget(): def build_app(monkeypatch): """Return a factory that builds the real app with GRAPHITI_API_KEY set to a given value. - The process environment should be the only input, and two things otherwise get a say: - Settings reads env_file='.env' relative to the cwd (server/ under `make test`), and - graphiti_core calls load_dotenv() on import, which searches upward from server/.venv and - finds the same file whatever the cwd. Either hands a developer with a server/.env the very - key test_no_key_means_no_service asserts the absence of, failing the suite on their machine - only. So: no env file for Settings, and the environment is set after the import that may - inject one. + The process environment should be the only input, and two things otherwise get a say: Settings + reads env_file='.env' relative to the cwd, and graphiti_core calls load_dotenv() on import, + which finds the same file whatever the cwd. Either hands a developer with a server/.env the + very key test_no_key_means_no_service asserts the absence of, failing the suite on their + machine only. So: no env file for Settings, and the environment is set after the import. """ monkeypatch.setitem(config.Settings.model_config, 'env_file', None) def _build(graphiti_api_key: str | None): - # Imported before the environment is arranged, because this is the import that - # triggers load_dotenv() — on the first call, which is the one that matters. Nothing - # reads settings yet: the app is assembled at import, get_settings() runs in lifespan. + # Imported before the environment is arranged, because this is the import that triggers + # load_dotenv(). Nothing reads settings yet: the app is assembled at import, and + # get_settings() runs in the lifespan. import graph_service.main as main monkeypatch.setenv('OPENAI_API_KEY', 'unused-by-these-tests') @@ -90,7 +87,7 @@ def _assert_passed_auth(response): """Assert only that require_api_key let the request through. Not `== 500`: what the handler does next is not this file's business, and pinning it would - break the day the fixture gains a lifespan. 401 is the one status meaning auth rejected it. + break the day the fixture gains a lifespan. """ assert response.status_code != 401, response.text @@ -99,13 +96,9 @@ def _assert_passed_auth(response): def test_no_key_means_no_service(build_app, graphiti_api_key): """Auth is mandatory, so a missing key is a startup error and not an open API. - A service that boots without a key and one that 401s everything both look healthy to Render, - and only one is safe. Blank and whitespace count as missing: a stray `GRAPHITI_API_KEY=` must - not become a key of '' or ' ' that the deployment accepts. - - Asserted against the lifespan rather than get_settings() alone, because that is what makes it - a startup failure — uvicorn runs it before serving, so the process exits. Entering it is - enough, since settings are read on the first line. + Blank and whitespace count as missing: a stray `GRAPHITI_API_KEY=` must not become a key of '' + that the deployment accepts. Asserted against the lifespan rather than get_settings() alone, + because that is what makes it a startup failure — uvicorn runs it before serving. """ app = build_app(graphiti_api_key) with pytest.raises(ValidationError, match='graphiti_api_key'), TestClient(app): @@ -115,10 +108,9 @@ def test_no_key_means_no_service(build_app, graphiti_api_key): def test_a_guessable_key_is_refused_at_startup(build_app): """A key too short to survive guessing fails the deploy, rather than serving traffic. - The budget stops a stranger working through the keyspace, not a key already at the front of - it. Rotation is a hand edit in the Dashboard, and nothing there would reject - `GRAPHITI_API_KEY=dev` — this is what does. One character under the floor, so the boundary is - pinned rather than a value that would pass a floor set lower by accident. + The budget stops a stranger working through the keyspace, not a key already at the front of it. + Rotation is a hand edit in the Dashboard, and nothing there rejects `GRAPHITI_API_KEY=dev` — + this is what does. One character under the floor, so the boundary itself is pinned. """ app = build_app('k' * (MIN_API_KEY_LENGTH - 1)) with pytest.raises(ValidationError, match='string_too_short'), TestClient(app): @@ -149,9 +141,8 @@ def test_configured_key_accepts_the_right_bearer_token(build_app): # The right value one byte short: guards against compare_digest becoming a prefix # comparison. pytest.param({'Authorization': f'Bearer {SECRET[:-1]}'}, id='truncated-key'), - # Raw bytes, since httpx refuses a non-ASCII str header. Starlette decodes as - # latin-1, so this arrives as a non-ASCII str, which compare_digest refuses to - # compare. Must be a 401, not a 500 for anyone sending a stray high byte. + # Raw bytes, since httpx refuses a non-ASCII str header. Starlette decodes as latin-1, + # so this arrives as a str compare_digest refuses to compare — a 401, not a 500. pytest.param({b'Authorization': b'Bearer \xff'}, id='non-ascii-token'), ], ) @@ -161,14 +152,14 @@ def test_configured_key_rejects_everything_else(build_app, headers): # Every value is over MIN_API_KEY_LENGTH, and the match below pins the pattern error: a short -# non-ASCII key would fail on length and pass without the ASCII rule existing at all. +# non-ASCII key would fail on length even with no ASCII rule at all. @pytest.mark.parametrize( 'graphiti_api_key', [ pytest.param('clé-secrète-assez-longue', id='accent'), pytest.param('key-with-an-emoji-🔑', id='emoji'), - # A tab survives RequiredStr's strip only mid-value, and a control character in a - # header is malformed however it is encoded. + # A tab survives RequiredStr's strip mid-value, and a control character in a header + # is malformed however it is encoded. pytest.param('key\twith\ttabs\tin\tit', id='control-char'), ], ) @@ -176,9 +167,8 @@ def test_a_non_ascii_key_is_refused_at_startup(build_app, graphiti_api_key): """A key that can't survive an HTTP header fails the deploy, rather than every request. Nothing in the Dashboard rejects a passphrase on rotation. Such a key authenticates for a - client encoding the header as latin-1 and not for one using UTF-8, so it can't be relied on - either way — and the operator who set it sees the same 401 as someone who mistyped it. - Failing at startup names the actual problem. + client encoding the header as latin-1 but not for one using UTF-8, and the operator who set it + sees the same 401 as someone who mistyped it. Failing at startup names the actual problem. """ app = build_app(graphiti_api_key) with pytest.raises(ValidationError, match='string_pattern_mismatch'), TestClient(app): @@ -189,7 +179,7 @@ def test_a_burst_of_wrong_keys_is_rate_limited(build_app): """The key can't be brute-forced, and the endpoint isn't a free scanner target. Exactly MAX_FAILED_AUTH rejections are spent first, pinning the boundary both ways: the last - inside the budget still gets its 401, the first past it does not. + inside the budget still gets its 401, the first past it doesn't. """ app = build_app(SECRET) wrong = {'Authorization': 'Bearer wrong-key-but-long-enough'} @@ -205,8 +195,8 @@ def test_a_burst_of_wrong_keys_is_rate_limited(build_app): def test_the_right_key_is_never_rate_limited(build_app): """Why a global budget is safe: it can't lock a real client out. - Otherwise one stranger guessing keys takes the API down for whoever holds the real one — a - worse outcome than the brute-force the budget exists to stop. + Otherwise one stranger guessing keys takes the API down for whoever holds the real one — worse + than the brute-force the budget exists to stop. """ app = build_app(SECRET) wrong = {'Authorization': 'Bearer wrong-key-but-long-enough'} @@ -250,10 +240,10 @@ def test_healthcheck_stays_public(build_app): @pytest.mark.parametrize('path', DOCS_PATHS) def test_the_docs_are_browsable(build_app, path): - """Deliberately public, and worth a test because it is a judgement call, not a default. + """Deliberately public, and worth a test because it's a judgement call, not a default. A browser can't attach a bearer header to a navigation, so protecting these would make them - unreachable by clicking a link on any deployment with a key. + unreachable by clicking a link. """ app = build_app(SECRET) assert TestClient(app).get(path).status_code == 200 @@ -262,8 +252,8 @@ def test_the_docs_are_browsable(build_app, path): def test_the_docs_offer_the_bearer_scheme(build_app): """Swagger's Authorize button, which keeps the docs usable against a deployment. - It comes from the routers' dependency, easy to lose by protecting the routes some other way, - and its absence would leave Try it out silently 401ing. + It comes from the routers' dependency — easy to lose by protecting the routes some other way, + which would leave Try it out silently 401ing. """ schema = TestClient(build_app(SECRET)).get('/openapi.json').json() assert schema['components']['securitySchemes']['HTTPBearer']['scheme'] == 'bearer' @@ -273,9 +263,9 @@ def test_the_docs_offer_the_bearer_scheme(build_app): def _is_protected(route) -> bool: """Whether require_api_key runs before this route's handler. - Identity, not a name comparison, so a same-named dependency elsewhere can't satisfy it. - Anything that is not an APIRoute has no dependant and counts as unprotected — a Mount or - WebSocketRoute is exactly what this must not wave through. + Identity, not a name comparison, so a same-named dependency elsewhere can't satisfy it. Anything + that isn't an APIRoute has no dependant and counts as unprotected — a Mount or WebSocketRoute is + exactly what this must not wave through. """ if not isinstance(route, APIRoute): return False @@ -285,8 +275,8 @@ def _is_protected(route) -> bool: def test_every_route_but_the_public_ones_is_protected(build_app): """The whole surface, not a list of prefixes someone has to remember to extend. - A router included without the dependency, or a public endpoint added by accident, fails - here; adding a route to PUBLIC_PATHS is then a deliberate edit. + A router included without the dependency, or a public endpoint added by accident, fails here; + adding a route to PUBLIC_PATHS is then a deliberate edit. """ app = build_app(SECRET) unprotected = { diff --git a/server/tests/test_live_falkordb_int.py b/server/tests/test_live_falkordb_int.py index 2b10936e4..e187ba077 100644 --- a/server/tests/test_live_falkordb_int.py +++ b/server/tests/test_live_falkordb_int.py @@ -50,9 +50,8 @@ pytestmark = [pytest.mark.integration] -# The key the live server is spawned with; auth is mandatory, so it needs one to boot. Pinned, -# not inherited: a root-.env GRAPHITI_API_KEY would reach the subprocess via load_dotenv above -# and 401 every request here. +# The key the live server is spawned with; auth is mandatory, so it needs one to boot. Pinned, not +# inherited: a root-.env key would reach the subprocess via load_dotenv and 401 every request here. API_KEY = 'srvtest-api-key-Rk4Wm2' AUTH_HEADERS = {'Authorization': f'Bearer {API_KEY}'}