Topic 15.4
Reviewing AI Frontend Code
In one line
A code review for an AI feature looks for a small set of bugs that work in every demo and fail in production: unescaped output, a missing abort, a stale closure, unbounded history. Each bug is shown running, with its symptom, and then the reviewed version.
Think of it like this
Checking a car before a long trip. Nobody checks that the car starts: it just did. You check the tyres, the oil, the spare wheel, the lights. A review of AI frontend code is the same: the code already works when you demo it. You are looking for what only breaks later, on a slow network, with a hostile text, after a long conversation.
Words you'll meet
New words in this topic, in plain English. Come back here whenever one feels fuzzy.
- Code review
- Reading someone else's change before it is merged, to find bugs and improve it.
- innerHTML
- A browser property that turns a string into real page elements. With untrusted text it is dangerous.
- Stale closure
- A function that remembers an old value of a variable from the time it was created, and keeps using it after the value changed.
- Functional update
setState((old) => new): React gives you the latest value, not the one captured when your function was created.- Unbounded
- Without a limit. An unbounded list or request grows until something breaks.
- 413
- The HTTP status "payload too large": the request body is bigger than the server accepts.
- Regression test
- A test written after a bug, so the bug cannot come back.
- Smell
- A pattern in code that is not a bug yet but often leads to one.
Step by step
01The review path
Follow this order. It finds the worst bugs first, and it is short enough to do on every pull request that touches an AI feature.
02Bug 1: the answer goes in as HTML
The first example shows a reply that repeats text from a document the customer pasted. It contains a link that looks like part of Mitra and points to an outside site. The buggy code writes the reply with innerHTML, so the link becomes real. The reviewed code writes it as text, so the same characters show up as characters. (A <script> tag added through innerHTML does not run, but other tags and attributes take effect, and some attributes, such as event handlers, can run code. This example uses the mildest case, a fake link, which is already enough to trick a customer.)
Review rule: any innerHTML, dangerouslySetInnerHTML, insertAdjacentHTML or Markdown option that allows raw HTML, fed by model output, is a finding (Topics 6.3 and 9.3).
03Bug 2: no abort, so an old answer wins
The second example asks a slow question and then a fast one. Without an abort, the slow request finishes last and writes its answer over the new one. The reviewed version aborts the previous request when a new one starts, and does not show the AbortError as a failure.
Review rule: every fetch that fills a state variable needs a signal, or an id check, or both (Topics 3.1 and 4.7).
04Bugs 3 and 4: the closure and the growing history
The third example has two parts. In the first, setAnswer(answer + piece) runs inside a loop after an await. The function captured answer when it started, which was empty, so every update replaces the text with just the newest piece. The answer ends as one word. The fix is setAnswer((old) => old + piece).
In the second part, a chat sends its whole history on every turn. The body grows with every turn until the server refuses it with 413. The reviewed version keeps the newest messages that fit and tells the user (Topic 4.3). Review rule: any array that only gets push and is sent or stored needs a limit.
05The smell list
Keep these in mind on every pull request. Each is something that is not always a bug, but deserves a question.
1. NEXT_PUBLIC_ or a key in the browser bundle. 2. A user id or role read from the request body. 3. key={index} on a chat list. 4. catch {} that swallows errors, or one that shows error.message to the user. 5. A useEffect that starts a stream and never cleans up. 6. A send button that is not disabled while a request runs. 7. A tool that changes data without an approval. 8. A prompt or full conversation written to the logs. 9. No max on input length. 10. A setTimeout used to "wait for the stream".
Try it yourself
- 1
Try other markup
In the first example change the reply to contain
<b>URGENT</b>instead of a link, and run it. The buggy version shows a bold word the model chose to emphasise, which the user cannot tell from your own design. The reviewed version shows the tags as characters. - 2
Release the gate first
In the second example make the server answer
slowat once: delete the lineif (q === "slow") await slowGate;. Both versions now show the right answer, because the slow question is no longer slow. A race needs an unlucky order, which is why a bug like this never appears on a fast laptop. - 3
Change how many messages are kept
In the third example change
conversation(4)toconversation(8). Now at most 7 messages are sent (8 kept, minus a leading answer), which is still under this made-up server limit, so nothing fails however long the chat runs. Then tryconversation(2): it never fails, but only the newest question is sent, so the model has forgotten everything before it. The limit is a choice between cost, memory and quality.
Code & diagrams
collector.example stands for any outside site. In the buggy version nothing looks wrong: the text reads naturally, and the link is clickable. In the reviewed version the user sees the characters, which makes the problem visible.
Preview
Console
Nothing logged yet.Expected console output
the reply has 1 link written in it as characters
buggy: links on the page: 1 | the user reads: "Your order is on its way. Sign in again to track it"
reviewed: links on the page: 0 | the user reads: "Your order is on its way. <a href=\"https://collector.example/login\">Sign in again to track it</a>"The held request is how the example forces the unlucky order. In real life the order depends on the network, so the bug appears only for some users, some of the time.
Preview
Console
Nothing logged yet.Expected console output
buggy: after the new question is answered: Answer to the fast question.
buggy: after the old request ends: Answer to the slow question.
reviewed: after the new question is answered: Answer to the fast question.
reviewed: after the old request ends: Answer to the fast question.Both bugs pass a quick test: one short question gets a good answer. The first needs a second token to show, the second needs a long conversation. The reviewed version keeps the newest 4 messages and then drops a leading assistant message, so what is sent starts with a question: that is why 3 are sent. The server limit (900 characters here) is made up; real limits are in tokens and are much larger, but the growth is the same.
Preview
Console
Nothing logged yet.Expected console output
buggy: the user reads: " days."
reviewed: the user reads: "Returns are accepted within seven days."
turn | unbounded history | newest 4 messages, from a question
turn 1: 200 (1 messages sent) | turn 1: 200 (1 messages sent)
turn 2: 200 (3 messages sent) | turn 2: 200 (3 messages sent)
turn 3: 200 (5 messages sent) | turn 3: 200 (3 messages sent)
turn 4: 200 (7 messages sent) | turn 4: 200 (3 messages sent)
turn 5: 413 | turn 5: 200 (3 messages sent)
turn 6: 413 | turn 6: 200 (3 messages sent)
turn 7: 413 | turn 7: 200 (3 messages sent)Break it on purpose
Errors are the best teachers. Make each change, read the error, guess what went wrong, then reveal the answer.
Break #1
Bring innerHTML back into the reviewed version
In the first example replace the line flushSync(() => createRoot(reviewed).render(<Answer text={reply} />)); with showBuggy(reviewed, reply); and press Run.
Myth vs fact
Myth
innerHTML is safe because browsers do not run scripts added that way.
Fact
True only for <script> tags. Other elements work (links, forms, images), and event-handler attributes on them can run code. A fake login link needs no script at all (Topic 6.3).
Myth
If the demo works, the code works.
Fact
AI code fails on timing and growth. Test with a slow first request, a stream cut in pieces, a second question during the first and a long conversation.
Myth
A functional update is a style choice.
Fact
Inside a loop or after an await, setX(x + 1) uses an old x. setX((old) => old + 1) is correct, and the other is a bug.
Myth
Limits on history are the server's problem.
Fact
The page decides what to send. A server limit protects the server, but a page that never trims will meet it as an error (Topic 4.3).
Pro corner
Extra depth for experienced readers. New to this? Skip it for now and come back later.
- ▸
Write the regression test with the bug: hostile reply rendered, first request finishing last, a stream in pieces, a 30-turn conversation. A review comment that includes the test gets fixed and stays fixed.
- ▸
Search the diff for these words first:
innerHTML,dangerouslySetInnerHTML,fetch(,setTimeout,NEXT_PUBLIC,console.log(,push(,key={i. Each is a place to ask a question. - ▸
Review the unhappy paths as seriously as the happy path: what the user reads when it fails (Topic 1.3), what happens at the limit (Topic 13.2), what the screen reader hears (Topic 10.6).
- ▸
Be kind and specific. Name the user-visible symptom, not the author's mistake.
Remember this
- 1
The problem this solves: AI features hide bugs well. The answer appears, the demo passes, and the bug waits for a second question, a slow connection, a long chat or a strange input. A reviewer who knows the usual hiding places finds them in minutes. This topic gives you the places to look, and a way to write the comment so the author fixes it.
- 2
Look at the data flow first. Follow one piece of text from the model to the screen, and one piece of user input to the model. On the way out ask: is it ever treated as HTML or as code? On the way in ask: is its size limited, is the user known, is anything secret added? Most serious bugs sit where text changes hands.
- 3
Then look at time. AI requests are slow, so time is where the bugs are. Ask: what if the user asks again while this one runs? What if the component unmounts? What if two answers finish out of order? What if the stream stops in the middle? Look for
AbortController, a request id, a cleanup function and an error branch for each. - 4
Then look at state and growth. Does the code build new state from old state with a function (
setX((old) => …)) when it runs inside a loop or after anawait? Is any list (history, logs, cache) allowed to grow without limit? Does the page send everything every time? - 5
Four bugs, run live in three examples. They show an unescaped answer put into the page with
innerHTML, a missing abort that lets a stale answer overwrite a new one, a stale closure that keeps only the last token, and an unbounded history that grows until the server refuses it. In each, the failure prints in the console, and the reviewed version prints next to it. Bugs that do not show here but are common: a stream started inuseEffectwithout cleanup, a key in the browser bundle, and a send button that stays enabled while a request runs. - 6
How to write the comment. Say what the user will see, how to reproduce it, the fix, and the test that would have caught it. "This can show a stale answer: ask a slow question and then a fast one. Abort the previous request when a new one starts, and ignore
AbortError. Add a test where the first request finishes last." A comment like that is hard to argue with and quick to act on.
Explain it without notes
What are the first five things you check when reviewing a pull request for an AI chat feature?
A pull request writes model output with dangerouslySetInnerHTML. Write the review comment.
Users report that sometimes they see the answer to their previous question. What do you look for in the code?
A streamed answer only ever shows its last word. What is the bug and the fix?
How do you review the way a chat keeps its history?
Practice
Review this snippet and write three comments: useEffect(() => { fetch('/api/chat?user=' + userId, { method: 'POST', body: JSON.stringify({ messages }) }).then(r => r.text()).then(setAnswer) }, [messages]).
Write a regression test, in words, for the stale-answer bug in the second live example.
Trade-offs
- ↔
A strict review slows a pull request but each bug found here would otherwise be found by a user, on a bad day, with no stack trace. Spend the time where text changes hands and where time matters.
- ↔
Review checklists find the known bugs. They do not replace tests that run the unlucky orders (slow first, split packets, long chat) in CI.
Done when you can
I can follow model text to the screen and spot an HTML sink.
I can spot a missing abort or id check and write the test for it.
I can explain a stale closure and the functional update that fixes it.
I can spot a history or list that only grows.
I can write a review comment with the symptom, the cause, the fix and the test.