This pull request introduces several enhancements and database schema changes to support associating LLM completions with prospects, improve API security, and add utility scripts for database management. #66
Conversation
Associate LLM records with prospects by adding a prospect_id foreign key. Updated create_table.sql to include prospect_id REFERENCES prospects(id), added an ALTER TABLE migration SQL and a small run script to apply it, and updated app/api/llm/llm.py to return and persist prospect_id in SELECT and INSERT operations. Removed a deprecated insert_llm_lorem.py test script.
Add a small CLI script (app/api/llm/sql/empty_llm_table.py) that connects to the database via get_db_connection_direct and executes `DELETE FROM llm;`. The script adjusts sys.path to import the app utils, opens a cursor, commits the deletion, closes the connection, and prints a confirmation. Note: this is a destructive operation intended to purge all rows from the llm table.
When reading a prospect, fetch related llm rows and attach them as data['llm_records'] (uses get_db_connection_direct and falls back to an empty list on error). Add SQL schema to create the prospects table (app/api/prospects/sql/create_table.sql) and a helper script to describe the prospects table columns (app/api/prospects/sql/describe_prospects_table.py). These changes expose related LLM data and provide the DB schema + inspection utility for the prospects resource.
Introduce API key authentication and a DB migration for LLM prompts. Changes include: added PYTHON_KEY to .env.sample; new app/utils/api_key_auth.py providing get_api_key (APIKeyHeader + dotenv) and used as a Depends in app/api/llm/llm.py; added SQL migration app/api/llm/sql/alter_add_prompt_code.sql and a runnable script app/api/llm/sql/run_alter_add_prompt_code.py to add a prompt_code TEXT column to the llm table; and a small reordering of the endpoints list in app/api/root.py to include the llm entry. These updates enable API key protected LLM endpoints and persist a prompt_code field in the database.
There was a problem hiding this comment.
Pull request overview
This PR adds database and API changes to associate LLM completions with prospects, introduce API-key protection for LLM retrieval, and include new DB utility/migration scripts.
Changes:
- Add
prospect_id(FK) andprompt_codecolumns tollm, plus a newprospectstable definition. - Update
/llmendpoints to read/writeprospect_id, and extend/prospects/{id}to include relatedllmrecords. - Introduce API-key auth helper and add maintenance scripts (empty table, describe schema).
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| app/utils/api_key_auth.py | Adds API key auth dependency used by /llm GET. |
| app/api/root.py | Reorders endpoint listing returned by /. |
| app/api/prospects/sql/describe_prospects_table.py | Adds a script to print prospects schema from information_schema. |
| app/api/prospects/sql/create_table.sql | Adds SQL bootstrap definition for the prospects table. |
| app/api/prospects/prospects.py | Extends prospect detail response to include related llm records. |
| app/api/llm/sql/run_alter_add_prospect_id.py | Adds runner script to migrate llm.prospect_id. |
| app/api/llm/sql/run_alter_add_prompt_code.py | Adds runner script to migrate llm.prompt_code. |
| app/api/llm/sql/insert_llm_lorem.py | Removes an older utility insert script. |
| app/api/llm/sql/empty_llm_table.py | Adds script to delete all records from llm. |
| app/api/llm/sql/create_table.sql | Updates llm bootstrap DDL to include prospect_id FK. |
| app/api/llm/sql/alter_add_prospect_id.sql | Adds SQL migration for prospect_id FK column. |
| app/api/llm/sql/alter_add_prompt_code.sql | Adds SQL migration for prompt_code column. |
| app/api/llm/llm.py | Adds API-key dependency to /llm GET and persists prospect_id on POST. |
| .env.sample | Adds PYTHON_KEY to sample env config. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| API_KEY_NAME = "X-API-Key" | ||
| api_key_header = APIKeyHeader(name=API_KEY_NAME, auto_error=False) | ||
|
|
||
| def get_api_key(request: Request, api_key_header: Optional[str] = ""): | ||
| if API_KEY is None: |
There was a problem hiding this comment.
get_api_key is not actually reading the X-API-Key header: the api_key_header parameter here is treated by FastAPI as a query parameter, and the APIKeyHeader(...) dependency defined above is never used. This makes the API key check trivially bypassable (e.g., via ?api_key_header=...) and breaks the intended header-based authentication. Use api_key: str | None = Depends(api_key_header)/Security(api_key_header) to extract the header value and validate that instead.
| return {"meta": meta, "data": []} | ||
| if api_key_header == API_KEY: | ||
| return api_key_header | ||
| else: | ||
| meta = make_meta("error", "Invalid PYTHON_KEY Key") | ||
| return {"meta": meta, "data": []} |
There was a problem hiding this comment.
On invalid/missing API key (or missing PYTHON_KEY env var), this dependency returns a normal dict response instead of raising an HTTPException (e.g., 401/403). Because the /llm handler continues executing after the dependency returns, requests without a valid key will still receive data. Change this to raise HTTPException(status_code=..., detail=...) so the request is rejected early.
| return {"meta": meta, "data": []} | |
| if api_key_header == API_KEY: | |
| return api_key_header | |
| else: | |
| meta = make_meta("error", "Invalid PYTHON_KEY Key") | |
| return {"meta": meta, "data": []} | |
| raise HTTPException( | |
| status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, | |
| detail=meta, | |
| ) | |
| if api_key_header == API_KEY: | |
| return api_key_header | |
| else: | |
| meta = make_meta("error", "Invalid PYTHON_KEY Key") | |
| raise HTTPException( | |
| status_code=status.HTTP_401_UNAUTHORIZED, | |
| detail=meta, | |
| ) |
| model TEXT | ||
| model TEXT, | ||
| prospect_id INTEGER REFERENCES prospects(id) | ||
| ); |
There was a problem hiding this comment.
Queries in prospects_read_one filter llm by prospect_id and order by id. Without an index on llm.prospect_id, this will devolve into sequential scans as the table grows. Add an index (e.g., on (prospect_id, id DESC) or at least prospect_id) via a migration/bootstrap SQL.
| ); | |
| ); | |
| CREATE INDEX IF NOT EXISTS llm_prospect_id_id_desc_idx | |
| ON llm (prospect_id, id DESC); |
| @@ -0,0 +1,2 @@ | |||
| -- Migration: Add prospect_id column to llm table | |||
| ALTER TABLE llm ADD COLUMN IF NOT EXISTS prospect_id INTEGER REFERENCES prospects(id); No newline at end of file | |||
There was a problem hiding this comment.
This migration adds a foreign key reference to prospects(id). If the prospects table does not exist yet in the target DB, this ALTER TABLE will fail. Consider creating prospects first (or splitting into: add column, then add FK constraint after verifying table existence) and document/encode the required migration order.
| ALTER TABLE llm ADD COLUMN IF NOT EXISTS prospect_id INTEGER REFERENCES prospects(id); | |
| ALTER TABLE llm ADD COLUMN IF NOT EXISTS prospect_id INTEGER; | |
| DO $$ | |
| BEGIN | |
| IF EXISTS ( | |
| SELECT 1 | |
| FROM information_schema.tables | |
| WHERE table_schema = current_schema() | |
| AND table_name = 'prospects' | |
| ) AND NOT EXISTS ( | |
| SELECT 1 | |
| FROM pg_constraint | |
| WHERE conname = 'llm_prospect_id_fkey' | |
| ) THEN | |
| ALTER TABLE llm | |
| ADD CONSTRAINT llm_prospect_id_fkey | |
| FOREIGN KEY (prospect_id) REFERENCES prospects(id); | |
| END IF; | |
| END $$; |
| data JSONB, | ||
| model TEXT | ||
| model TEXT, | ||
| prospect_id INTEGER REFERENCES prospects(id) |
There was a problem hiding this comment.
The new prompt_code column is added via migrations in this PR, but it is missing from the base CREATE TABLE definition. Fresh environments created from create_table.sql will not have prompt_code unless migrations are also run, causing schema drift. Add prompt_code to this table definition to keep bootstrap and migrations consistent.
| prospect_id INTEGER REFERENCES prospects(id) | |
| prospect_id INTEGER REFERENCES prospects(id), | |
| prompt_code TEXT |
| except Exception as llm_exc: | ||
| data["llm_records"] = [] |
There was a problem hiding this comment.
The except Exception as llm_exc: here silently drops the error and returns an empty list, which makes production failures to fetch llm_records hard to detect. At minimum, log llm_exc (and still return an empty list) so this doesn’t fail invisibly.
| prompt = payload.get("prompt") | ||
| prospect_id = payload.get("prospect_id") | ||
| if not prompt: | ||
| raise HTTPException(status_code=400, detail="Missing 'prompt' in request body.") |
There was a problem hiding this comment.
prospect_id is accepted from the request body but isn’t validated/cast. With the new FK constraint, non-integer values or non-existent IDs will make the insert fail later. Validate that prospect_id is an int (and optionally that the prospect exists) before attempting the insert.
| raise HTTPException(status_code=400, detail="Missing 'prompt' in request body.") | |
| raise HTTPException(status_code=400, detail="Missing 'prompt' in request body.") | |
| if prospect_id is not None: | |
| try: | |
| prospect_id = int(prospect_id) | |
| except (TypeError, ValueError): | |
| raise HTTPException(status_code=400, detail="Invalid 'prospect_id' in request body. It must be an integer.") |
| page_size: int = Query(10, ge=1, le=100, description="Records per page") | ||
| , api_key: str = Depends(get_api_key) |
There was a problem hiding this comment.
The parameter list formatting here (line break followed by a leading comma) is inconsistent and will be reformatted/flagged by common linters/formatters. Move the comma to the end of the previous line so parameters are consistently formatted.
| page_size: int = Query(10, ge=1, le=100, description="Records per page") | |
| , api_key: str = Depends(get_api_key) | |
| page_size: int = Query(10, ge=1, le=100, description="Records per page"), | |
| api_key: str = Depends(get_api_key) |
| @router.get("/llm") | ||
| def get_llm_records( | ||
| request: Request, | ||
| page: int = Query(1, ge=1, description="Page number (1-based)"), | ||
| page_size: int = Query(10, ge=1, le=100, description="Records per page") | ||
| , api_key: str = Depends(get_api_key) | ||
| ) -> dict: |
There was a problem hiding this comment.
If API-key auth is meant to be enforced, tests should cover both the unauthorized case (missing/invalid key -> 401/403) and the authorized case (valid X-API-Key -> 200). Right now this route can’t be safely tightened without updating/adding tests.
| cur = conn.cursor() | ||
| cur.execute( | ||
| """ | ||
| INSERT INTO llm (prompt, completion, duration, data, model) | ||
| VALUES (%s, %s, %s, %s, %s) | ||
| INSERT INTO llm (prompt, completion, duration, data, model, prospect_id) | ||
| VALUES (%s, %s, %s, %s, %s, %s) | ||
| RETURNING id; | ||
| """, | ||
| (prompt, completion, duration, data_blob, used_model) | ||
| (prompt, completion, duration, data_blob, used_model, prospect_id) | ||
| ) |
There was a problem hiding this comment.
This insert can now fail due to the new prospect_id FK (e.g., invalid/non-existent prospect), but failures are only logged and the endpoint still returns a "success" response. Consider validating prospect_id up front and/or returning an error (or clearly signaling persistence failure) when the DB insert fails so callers don’t get a misleading success result.
The most important changes are grouped below.
Database schema changes:
prospect_idcolumn to thellmtable, allowing each LLM completion to be linked to a prospect. This includes migration scripts (alter_add_prospect_id.sql,run_alter_add_prospect_id.py) and updates to the table creation SQL (create_table.sql). [1] [2] [3]prompt_codecolumn to thellmtable with corresponding migration scripts (alter_add_prompt_code.sql,run_alter_add_prompt_code.py). [1] [2]prospectstable definition for storing detailed prospect information.API and backend logic updates:
prospect_idfield: LLM records now includeprospect_idin both retrieval and insertion, and the/llmGET endpoint requires API key authentication. [1] [2] [3] [4] [5] [6]API security:
api_key_auth.py). The API expects aPYTHON_KEYenvironment variable and validates requests via theX-API-Keyheader. [1] [2]Utility and maintenance scripts:
llmtable (empty_llm_table.py) and describing theprospectstable schema (describe_prospects_table.py). [1] [2]insert_llm_lorem.pyscript.Minor updates:
These changes collectively enhance the system's ability to track LLM completions by prospect, improve API security, and facilitate database maintenance.