2026-09-07 · 9 min read

LLM app code review: 15 Python exercises on the OpenAI SDK

In April 2026, Noma Security disclosed a vulnerability they called GrafanaGhost (CyberScoop's report has Grafana's response). Grafana's AI assistant reads dashboard content to help operators, which sounds perfectly reasonable until you consider what happens when someone puts instructions inside that content. The researchers planted instructions in data Grafana stores and later feeds to the assistant, and once that text was in the prompt the model did what language models do with instructions: it followed them. In their proof of concept it wrote sensitive data into a Markdown image whose protocol-relative URL slipped past the domain check, so rendering the image sent the data to their server. Grafana Labs disputed the severity, said the exploit needed significant user interaction, and confirmed no Grafana Cloud data was leaked. Nobody broke encryption. Nobody guessed credentials. The content was already inside the prompt, the prompt was the instruction channel, and nobody had drawn a line between the two.

That is one bug class. LLM applications built on the OpenAI SDK have at least a dozen more, and the uncomfortable thing about most of them is that they do not look like bugs at all. The code is clean, the types check, the demo works beautifully on the three inputs anyone ever tries, and the defect sits in an assumption that no linter will flag and no test will catch because the test makes the same assumption. The only thing that catches it is a reviewer who reads the requirement, reads the code, and notices the gap between the two. We built fifteen Python PRs around these patterns and added them to the DiffDojo library as a review track: twelve with one planted bug each, and three that are deliberately clean, because knowing when to approve is half the skill. Every PR uses the openai SDK 1.x directly, no LangChain, no abstraction layer, just the shapes you actually see at work. The code was drafted by AI (qwen3.8-max and deepseek-v4-pro as scenario co-authors), with each defect designed by us. What follows is five of the twelve mechanisms, enough to build the instinct. The library is the exercise.

Retrieved text in the system prompt

Start with the GrafanaGhost shape, because it is the one most teams will build without realizing they built it. A support-chat endpoint retrieves a help-center article and grounds the assistant's answer in it. Four lines, reads clean:

The requirement says: system prompt is a constant; article text enters only as a user message. The code does the opposite. The article body, written by hundreds of support agents, edited by contractors, sometimes scraped from external sources, gets f-string-interpolated straight into the system message. That is the GrafanaGhost shape: data and instructions sharing one channel, so any instruction embedded in the data becomes an instruction to the model. It does not matter that the articles "should" be safe. The content is untrusted by origin, and interpolating it into the system message gives every author and editor of every article the ability to override the assistant's behavior. The requirement drew the line in the right place. The code erased it. The fix is structural, not a filter: article text goes in a user-role message, never the system message, because the boundary between the two is the only thing that separates "here is context" from "here is what to do."

Unvalidated tool-call arguments

This one is subtler, because it requires you to think about who the caller actually is. A support assistant can call an order-lookup tool, and the model decides when to invoke it and what arguments to pass:

The requirement says: tool args must be validated against the schema before the tool runs. The code parses the model's JSON and spreads it directly into a database-backed function with **args. No schema check, no type coercion, no allow-list of keys. The instinct that catches this is recognizing that the model is not a trusted internal caller. It is a probabilistic text generator that produces structurally valid JSON which can contain extra keys, wrong types, values outside the expected domain, or fields the function never intended to accept. When you see function.arguments flowing into anything with side effects, apply the same reflex you would apply to a request body from an anonymous HTTP client: validate the shape before you use it, because the author of that JSON is not on your team.

The unit mismatch nobody tests for

This is the kind of bug that makes you feel foolish once you see it, precisely because it is so simple that nobody writes a test for it. A batch ticket summariser backs off when rate-limited. The provider returns a retry-after-ms header, and the code computes how long to sleep:

The return value feeds asyncio.sleep(), which takes seconds. The header value is milliseconds. An 800ms backoff hint becomes an 800-second stall, over thirteen minutes, and nothing crashes or raises or logs a warning. The batch just takes fifty times longer than it should, and the team assumes the provider is slow tonight. The unit test that mocks retry-after-ms: 1000 produces a sleep of 1000 seconds instead of 1 second, which looks like a timeout bug rather than a unit bug, so someone adds a smaller mock value and moves on. The review catch here is mechanical: when a value carries its unit in its name, follow the name to the API that consumes it and check whether they agree. The header says milliseconds. The function says seconds. The missing / 1000 is the entire bug.

Retry that forgets what already happened

This one moves money, which makes it the scariest bug in the track even though the code looks like a perfectly reasonable retry loop. A refund agent retries the model call on timeout. The payments service has no idempotency key, so every accepted call is final. Inside the retry loop:

The message list is rebuilt from scratch on every attempt. Think about what that means when the first attempt timed out. The timeout happened on the network side, but the model may have already called the refund tool, and the payment service may have already accepted it. The money moved. Now the retry starts over with a fresh message list, no record of the refund that already ran. There is a guard function that checks the conversation history for a prior refund call, but the history was just rebuilt from the system prompt and the user text, so the guard sees nothing and allows the refund again. The customer gets their money back twice. The requirement says it plainly: refund at most once per request; retry on timeout preserves messages. The code rebuilds them inside the loop instead of building them once before it, and that one placement decision is the difference between a retry that remembers and a retry that forgets.

Filtered results zipped against unfiltered IDs

The last example is a data-alignment bug, the kind that produces no error and no crash, just quietly wrong answers. A support assistant executes multiple tool calls in parallel and needs to pair each result back to the tool_call_id it came from:

Picture three tool calls returning results. The second one fails. The filter on line 72 removes it from the list. Now successes has two items and tool_calls still has three, and zip on line 73 pairs them by position: the first result lands on the right ID, but the third result slides up into the second slot and gets paired with the wrong tool call's ID. Every result after the first failure is attributed to the wrong tool, silently, with no error. The model receives tool results that confidently answer the wrong question, and its next response is built on that foundation. On the happy path, when all three calls succeed, the lists stay aligned and everything works perfectly, which is why no demo and no unit test ever triggers this.

Seven more, and three that are fine

The five above are not the full set. The remaining seven mechanisms cover the other shapes LLM-app bugs take: a chat history that grows forever because nobody counts tokens against the context window, so the assistant works on short threads and crashes on long ones; a streaming handler that catches a mid-stream error, logs it, and then falls through to the completion path, yielding a done frame and persisting partial text as the complete answer, so the customer-support widget shows half a sentence and records it as the official response; a max_tokens budget derived from the input length instead of using the fixed constant the spec provides, so the reply shrinks as documents grow until long transcripts get one-word summaries; a 400 error message that helpfully interpolates the server's own OPENAI_API_KEY into the detail field to explain which key the user should not confuse with their own; a floating model alias (gpt-4o) where the requirement pins a dated snapshot, handing the decision of which model runs your eval suite to the provider's deployment schedule; tool results appended before the assistant's tool_calls turn in the message history, which locally makes sense ("here's the answer, then the question") but violates the message-ordering contract and produces an invalid transcript; and a batch loop with no try/except inside, so the first row's RateLimitError kills the entire fifty-row request and the caller gets a 500 instead of the per-row report the response schema promises.

Each of those maps to a single question you can ask while reading the diff. Does this context window have a ceiling? Can the client tell "that was the whole answer" from "the answer was cut off"? Who controls which model version runs, us or the provider? Does this error payload contain anything only the server should know? The questions are short. The hard part is remembering to ask them at 4pm on the eleventh PR of the day, when the code looks clean and the demo worked and merging is one click away.

Three of the fifteen PRs have nothing wrong with them. A ticket classifier that keeps untrusted email text in the user message where it belongs, parses the model's output defensively, and gets the token arithmetic right. An SSE streaming chat that emits proper error frames and never persists partial output. A weather tool agent with complete schema validation, correct message ordering, and an idempotency guard. They are in the track because approving clean code is a skill, and a reviewer who requests changes on everything is not careful, they are noisy, and teams learn to ignore them. The hardest moment in a review is reading a diff that handles every edge case you can think of, checking it against the requirements, finding nothing wrong, and saying "approve" without hedging. If you cannot defend an approve with the same confidence you request changes, the muscle is not trained yet. Three of these fifteen will test that.

What these have in common

LLM application code has a surface area that traditional backend code does not: the model is a caller you do not control, the context window is a budget you must manage, the message transcript is a protocol with ordering rules, and every piece of retrieved content is a potential instruction. These are not exotic concerns. They are the ordinary mechanics of any application built on the chat completions API, and every one of them produces bugs that pass the demo and fail in production.

Every bug in this track shares one property: it is invisible on the happy path. The prompt injection only fires when a specific article contains instructions. The unit mismatch only matters when the provider actually rate-limits. The retry only double-refunds when the first attempt times out after the tool call succeeds. The positional zip only breaks when at least one tool call fails. These are all second-run, second-input, second-path bugs, and they survive review because the first run, the one the demo uses, works perfectly.

The reviewing reflex that catches them is not domain-specific. It is the same question, applied to LLM-specific surfaces: what is this code assuming, and did anyone actually promise that? The system prompt assumes the article is safe. The retry assumes the first attempt had no side effects. The zip assumes all results are present. The sleep assumes the header and the API use the same unit. None of those assumptions are stated in the code. All of them are wrong.